Merge pull request #3 from nginx-shib/remove-flags

Remove the shib_authorizer flag on the config, update readme

README → README.rst

 Shibboleth auth request module for nginx
 ========================================
 
-This module allows authorization based on the result of a subrequest to Shibboleth.
-Once a subrequest returns 2xx status - access is allowed; on 401 or 403 - 
-access is disabled with an appropriate status.  
+This module allows authorization based on the result of a subrequest to
+Shibboleth.  Once a subrequest returns 2xx status - access is allowed; on 401
+or 403 - access is disabled with an appropriate status.
 
-For 401 statuses, the WWW-Authenticate header from the subrequest response 
-will be passed to client.
+For 40x statuses, the WWW-Authenticate header from the subrequest response
+will be passed to client.  All other subrequest response statuses (such as 3xx
+redirects) are passed back to the client, including status and headers.  This
+mostly conforms to the FastCGI Authorizer specification, with the exception of
+the processing of the sub-request and sub-response bodies due to limitations
+in Nginx. As the Shibboleth FastCGI authorizer does not consider the contents
+of a request body or use response bodies, this is not an issue.
 
-All other subrequest response statuses are considered to be an error, unless
-the `shib_authorizer=on` flag is supplied, in which case the module will
-return the subrequest's response status and headers. This mostly follows
-the FastCGI Authorizer specification, with the exception of the processing
-of the request and response bodies. Further information follows below.
-
-The module works at access phase and therefore may be combined nicely with other
-access modules (access, auth_basic) via satisfy directive.
+The module works at access phase and therefore may be combined nicely with
+other access modules (access, auth_basic) via satisfy directive.
 
 
 Configuration directives
 ========================
 
-    shib_request <uri>|off [flags]
+.. warning::
+
+   The ``shib_request`` directive no longer requires the ``shib_authorizer``
+   flag.  This must be removed for Nginx to start. No other changes are
+   required.
+
+::
+
+    shib_request <uri>|off
 
         Context: http, server, location
         Default: off
 
-        Switches Shibboleth auth request module on and sets uri which will be 
-        asked for authorization.
-
-        Flags may be configured to modify the behaviour of the module as
-        follows:
-        
-         * shib_authorizer=on - Configures the auth request module to explicitly
-           return the status, headers, and content of the response resulting
-           from the sub-request to the configured uri.
- 
-           This option allows a uri to conform to the FastCGI Authorizer
-           specification; see http://www.fastcgi.com/drupal/node/22#S6.3.
-           The one (potentially significant) caveat is that due to the way
-           Nginx operates at present with regards to subrequests (what
-           an Authorizer effectively requires), the request body will *not* be
-           forwarded to the authorizer, and similarly, the response body from
-           the authorizer will *not* be returned to the client. 
-
-           Configured URIs are not restricted to using a FastCGI backend
-           to generate a response, however.  This may be useful during
-           testing or otherwise, as you can use Nginx's built in ``return``
-           and ``rewrite`` directives to produce a suitable response.
+        Switches the Shibboleth auth request module on and sets uri which will be 
+        asked for authorization.  The configured uri should refer to a Nginx
+        location block that points to your Shibboleth FastCGI authorizer.
+
+        The HTTP status and headers of the response resulting
+        from the sub-request to the configured uri will be returned to the user,
+        in accordance with the FastCGI Authorizer
+        specification; see http://www.fastcgi.com/drupal/node/22#S6.3.
+        The one (potentially significant) caveat is that due to the way
+        Nginx operates at present with regards to subrequests (what
+        an Authorizer effectively requires), the request body will *not* be
+        forwarded to the authorizer, and similarly, the response body from
+        the authorizer will *not* be returned to the client. 
+
+        Configured URIs are not restricted to using a FastCGI backend
+        to generate a response, however.  This may be useful during
+        testing or otherwise, as you can use Nginx's built in ``return``
+        and ``rewrite`` directives to produce a suitable response.
+        Additionally, this module may be used with *any* FastCGI
+        authorizer, although operation may be affected by the above caveat.
 
     shib_request_set <variable> <value>
 
@@ -61,6 +66,8 @@ Configuration directives
 Usage
 =====
 
+::
+
     # FastCGI authorizer for Shibboleth Auth Request module
     location = /shibauthorizer {
         internal;
@@ -68,17 +75,16 @@ Usage
         fastcgi_pass unix:/opt/shibboleth/shibauthorizer.sock;
     }
 
-
     # A secured location. All incoming requests query the Shibboleth FastCGI authorizer.
     # Watch out for performance issues and spoofing.
     location /secure {
         more_clear_input_headers 'Variable-*' 'Shib-*' 'Remote-User' 'REMOTE_USER' 'Auth-Type' 'AUTH_TYPE';
-        
+
         # Add your attributes here. They get introduced as headers
         # by the FastCGI authorizer so we must prevent spoofing.
         more_clear_input_headers 'displayName' 'mail' 'persistent-id';
-        
-        shib_request /shibauthorizer shib_authorizer=on;
+
+        shib_request /shibauthorizer;
         proxy_pass http://localhost:8080;
     }
 
@@ -86,8 +92,11 @@ Usage
 Misc
 ====
 
-To compile nginx with this module, use "--add-module <path>" option
-to nginx configure.
+To compile nginx with this module, use the::
+
+    --add-module <path>
+
+option when you ``configure`` nginx.
 
 For further information on why this is a dedicated module, see
 http://forum.nginx.org/read.php?2,238523,238523#msg-238523

ngx_http_shibboleth_module.c

@@ -21,7 +21,6 @@
 
 typedef struct {
     ngx_str_t                 uri;
-    ngx_uint_t                authorizer;
     ngx_array_t              *vars;
 } ngx_http_auth_request_conf_t;
 
@@ -60,7 +59,7 @@ static char *ngx_http_auth_request_set(ngx_conf_t *cf, ngx_command_t *cmd,
 static ngx_command_t  ngx_http_auth_request_commands[] = {
 
     { ngx_string("shib_request"),
-      NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_1MORE,
+      NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE1,
       ngx_http_auth_request,
       NGX_HTTP_LOC_CONF_OFFSET,
       0,
@@ -113,7 +112,7 @@ ngx_http_auth_request_handler(ngx_http_request_t *r)
 {
     ngx_uint_t                    i;
     ngx_list_part_t               *part;
-    ngx_table_elt_t               *h, *ho, *hi;
+    ngx_table_elt_t               *h, *hi;
     ngx_http_request_t            *sr;
     ngx_http_post_subrequest_t    *ps;
     ngx_http_auth_request_ctx_t   *ctx;
@@ -145,121 +144,79 @@ ngx_http_auth_request_handler(ngx_http_request_t *r)
         }
 
         /*
-         * if authorizer mode is configured, handle the subrequest
+         * Handle the subrequest
          * as per the FastCGI authorizer specification.
          */ 
-        if (arcf->authorizer) {
-            ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
-                           "shib request authorizer handler");
-            sr = ctx->subrequest;
-
-            if (ctx->status == NGX_HTTP_OK) {
-                /* 
-                 * 200 response may include headers prefixed with `Variable-`
-                 * back into initial headers
-                 */
-                ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
-                           "shib request authorizer allows access");
-
-                part = &sr->headers_out.headers.part;
-                h = part->elts;
-
-                for (i = 0; /* void */; i++) {
-
-                    if (i >= part->nelts) {
-                        if (part->next == NULL) {
-                            break;
-                        }
-
-                        part = part->next;
-                        h = part->elts;
-                        i = 0;
-                    }
-
-                    if (h[i].hash == 0) {
-                        continue;
-                    }
-
-                    if (ngx_strncasecmp(h[i].key.data,
-                        (u_char *) "Variable-", 9) == 0) {
-                        /* copy header into original request */
-                        hi = ngx_list_push(&r->headers_in.headers);
-
-                        if (hi == NULL) {
-                            return NGX_HTTP_INTERNAL_SERVER_ERROR;
-                        }
-
-                        /* Strip the Variable- prefix */
-                        hi->key.len = h[i].key.len - 9;
-                        hi->key.data = h[i].key.data + 9;
-                        hi->value = h[i].value;
-
-                        hi->lowcase_key = ngx_pnalloc(r->pool, hi->key.len);
-                        if (hi->lowcase_key == NULL) {
-                            return NGX_HTTP_INTERNAL_SERVER_ERROR;
-                        }
-                        ngx_strlow(hi->lowcase_key, hi->key.data, hi->key.len);
-
-                        ngx_log_debug2(
-                          NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
-                          "shib request authorizer copied header: \"%V: %V\"",
-                          &hi->key, &hi->value);
-                    }
-                }
-                
-                return NGX_OK;
-            }
+        ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
+                       "shib request authorizer handler");
+        sr = ctx->subrequest;
 
+        if (ctx->status == NGX_HTTP_OK) {
             /* 
-             * Unconditionally return subrequest response status, headers 
-             * and content.
+             * 200 response may include headers prefixed with `Variable-`
+             * back into initial headers
              */
             ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
-                           "shib request authorizer returning sub-response");
+                       "shib request authorizer allows access");
 
-            r->headers_out = sr->headers_out;
-            return ctx->status;
-        }
-
-        /* return appropriate status */
-
-        if (ctx->status == NGX_HTTP_FORBIDDEN) {
-            return ctx->status;
-        }
+            part = &sr->headers_out.headers.part;
+            h = part->elts;
 
-        if (ctx->status == NGX_HTTP_UNAUTHORIZED) {
-            sr = ctx->subrequest;
+            for (i = 0; /* void */; i++) {
 
-            h = sr->headers_out.www_authenticate;
+                if (i >= part->nelts) {
+                    if (part->next == NULL) {
+                        break;
+                    }
 
-            if (!h && sr->upstream) {
-                h = sr->upstream->headers_in.www_authenticate;
-            }
+                    part = part->next;
+                    h = part->elts;
+                    i = 0;
+                }
 
-            if (h) {
-                ho = ngx_list_push(&r->headers_out.headers);
-                if (ho == NULL) {
-                    return NGX_ERROR;
+                if (h[i].hash == 0) {
+                    continue;
                 }
 
-                *ho = *h;
+                if (ngx_strncasecmp(h[i].key.data,
+                    (u_char *) "Variable-", 9) == 0) {
+                    /* copy header into original request */
+                    hi = ngx_list_push(&r->headers_in.headers);
 
-                r->headers_out.www_authenticate = ho;
-            }
+                    if (hi == NULL) {
+                        return NGX_HTTP_INTERNAL_SERVER_ERROR;
+                    }
 
-            return ctx->status;
-        }
+                    /* Strip the Variable- prefix */
+                    hi->key.len = h[i].key.len - 9;
+                    hi->key.data = h[i].key.data + 9;
+                    hi->value = h[i].value;
 
-        if (ctx->status >= NGX_HTTP_OK
-            && ctx->status < NGX_HTTP_SPECIAL_RESPONSE)
-        {
+                    hi->lowcase_key = ngx_pnalloc(r->pool, hi->key.len);
+                    if (hi->lowcase_key == NULL) {
+                        return NGX_HTTP_INTERNAL_SERVER_ERROR;
+                    }
+                    ngx_strlow(hi->lowcase_key, hi->key.data, hi->key.len);
+
+                    ngx_log_debug2(
+                      NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
+                      "shib request authorizer copied header: \"%V: %V\"",
+                      &hi->key, &hi->value);
+                }
+            }
+            
             return NGX_OK;
         }
 
-        ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
-                      "shib request unexpected status: %d", ctx->status);
+        /*
+         * Unconditionally return subrequest response status, headers
+         * and content.
+         */
+        ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
+                       "shib request authorizer returning sub-response");
 
-        return NGX_HTTP_INTERNAL_SERVER_ERROR;
+        r->headers_out = sr->headers_out;
+        return ctx->status;
     }
 
     ctx = ngx_pcalloc(r->pool, sizeof(ngx_http_auth_request_ctx_t));
@@ -293,8 +250,8 @@ ngx_http_auth_request_handler(ngx_http_request_t *r)
     }
 
     /* 
-     * true FastCGI authorizers should conditionally return the subrequest 
-     * response body but the FastCGI handler does not support
+     * true FastCGI authorizers should always return the subrequest 
+     * response body but the Nginx FastCGI handler does not support
      * NGX_HTTP_SUBREQUEST_IN_MEMORY at present.
      */
     sr->header_only = 1;
@@ -452,8 +409,7 @@ ngx_http_auth_request(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)
 {
     ngx_http_auth_request_conf_t *arcf = conf;
 
-    ngx_uint_t       i;
-    ngx_str_t        *value, s;
+    ngx_str_t        *value;
 
     if (arcf->uri.data != NULL) {
         return "is duplicate";
@@ -470,18 +426,6 @@ ngx_http_auth_request(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)
 
     arcf->uri = value[1];
 
-    for (i = 2; i < cf->args->nelts; i++) {
-
-        if (ngx_strncmp(value[i].data, "shib_authorizer=", 16) == 0) {
-            s.len = value[i].len - 16;
-            s.data = value[i].data + 16;
-            if (ngx_strcmp(s.data, "on") == 0) {
-                arcf->authorizer = 1;
-            }
-        }
-    }
-
-
     return NGX_CONF_OK;
 }