Writing a new auth module - request for comments

musicdenotation · June 17, 2015, 11:26am

Hi,

I’m writing a new module (out-of-tree) for supporting authentication
using Stormpath’s user management API (https://stormpath.com/).

Basically, the module makes one or more HTTP requests to the
Stormpath API to determine if the client request should be authorized
to access a location or not.

Since this is somewhat different than other modules I could learn from,
and
since all my knowledge about nginx internals is from looking at how
other
modules & core is written, I’m wondering if anyone could comment on how
I
designed the module and raise any issues if I did anything problematic,
wrong or weird.

For reference, the work-in-progress code for the module is available
here: GitHub - stormpath/stormpath-nginx-module: This is a deprecated module. Please see: https://github.com/stormpath/stormpath-nginx

Since I have to contact the external API I’m using the upstream module
to
do it. But I don’t want the users (admins) to have to define an upstream
block in nginx.conf so my module creates and configures an upstrem
configuration internally instead.

github.com

stormpath/stormpath-nginx-module/blob/master/src/ngx_http_auth_stormpath_module.c#L864


      
          
          
b->temporary = 1;
          
          
sr->request_body->bufs = ngx_alloc_chain_link(parent->pool);
          if (sr->request_body->bufs == NULL) {
              return NULL;
          }
          
          
sr->request_body->bufs->buf = b;
          sr->request_body->bufs->next = NULL;
          sr->request_body->buf = b;
          
          
sr->header_in = parent->header_in;
          sr->internal = 1;
          sr->uri = uri;
          
          
ctx->subrequest = sr;
          
          
ngx_http_set_ctx(parent, ctx, ngx_http_auth_stormpath_module);
          
          
/* We create the Stormpath API upstream internally instead of asking the

I haven’t seen any other module do that, but I don’t see that
it’s possible to avoid users having to define upstream manually
otherwise.

For the above reasons (wanting to handle everything invisible to the
user),
I’m not using nginx_http_proxy_module, but implement the upstream
handler
(create_request & friends) myself. But since I have to construct a HTTP
request, parse status line, parse headers, parse body (eg. if it’s
chunked
transfer-encoding), I end up duplicating a lot of functionality already
in http proxy (although greatly simplified because I know exactly how to
talk to the upstream server and what to expect in return).

One example is I parse the headers manually, because I haven’t found a
way
to init the http_upstream header parser hash, and to reuse the parser
(originally the init is done in ngx_http_upstream_init_main_conf).

github.com

stormpath/stormpath-nginx-module/blob/master/src/ngx_http_auth_stormpath_module.c#L304


      
          static ngx_int_t
          ngx_http_auth_stormpath_process_status_line(ngx_http_request_t *r)
          {
              ngx_int_t         rc;
              ngx_http_status_t status;
          
          
    ngx_memzero(&status, sizeof(ngx_http_status_t));
          
          
    rc = ngx_http_parse_status_line(r, &r->upstream->buffer, &status);
              if (rc == NGX_AGAIN) {
                  return rc;
              }
          
          
    if (rc == NGX_OK) {
                  r->upstream->headers_in.status_n = status.code;
                  r->upstream->process_header = ngx_http_auth_stormpath_process_header;
                  return ngx_http_auth_stormpath_process_header(r);
              }
          
          
    return NGX_HTTP_UPSTREAM_INVALID_HEADER;
          }

(I’ll also hit similar problems with caching the requests to the
upstream.
I’d like to reuse the caching functionality already in nginx, but it
seems
to me like http_proxy_module does a lot of manual heavy lifting in that
regard that I’d have to reimplement (or shudder copy-paste) to
support it?)

Does the above make sense? Is there an obvious way to do it differently
that
I’ve missed? Are there any guides or documentation on how this should be
done (besides Evan M.'s obsolete-but-useful guides I went through
already)?

Any comments, suggestions, warnings or flames are welcome.

Thanks,
Senko

Senko_R · June 17, 2015, 5:30pm

On Wednesday 17 June 2015 11:25:33 Senko R. wrote:

Hi,

I’m writing a new module (out-of-tree) for supporting authentication
using Stormpath’s user management API (https://stormpath.com/).

Basically, the module makes one or more HTTP requests to the
Stormpath API to determine if the client request should be authorized
to access a location or not.

[…]

Have you checked the auth_request module?

See: Module ngx_http_auth_request_module

wbr, Valentin V. Bartenev

Senko_R · June 18, 2015, 9:42am

Hi,

thanks for your reply Valentin. I have checked auth_request module, in
fact
the module I’m writing started as modifications to auth_request module.

To clarify, I’m not trying to do one-off setup for my server using the
Stormpath API. The idea is to provide a module so any Stormpath’s user
can
easily integrate the two.

Specifically, the reasons why auth_request wasn’t enough:

It requires another location on the local server to be provided (that
location can be proxied using http_proxy_module, but still has to be
added) to which it’ll make the requests. I wanted to avoid forcing the
users to need to add another location block and proxy_pass directives
to
the external API (felt like a hack).
It requires specific semantics regarding the response (200, 401, 403
are interpreted as usual, everything else is server error).
Stormpath’s
API has different semantics so it wouldn’t work anyways.
You can’t do more than one auth request per client request. In some
cases, I need two - first to authenticate the client, then to check if
the user is in a specific group (and to be able to do this, I need to
parse the response body).

So it looks like auth_request module would be ideal if the users provide
a small authorization web service that does whichever auth logic is
needed,
and then responds according to auth_request semantics. If I just wanted
to
implement the integration for my (one) specific use-case, I’d likely do
that.

But the motivation for the module is to avoid forcing users to do these
one-off auth services, and instead just compile in and use a module that
provides this.

Best,
Senko

On Wed, Jun 17, 2015 at 5:29 PM, Valentin V. Bartenev [email protected]
wrote:

[…]
nginx Info Page
–
Senko R.

Senko_R · June 18, 2015, 12:33pm

Hi Senko,

I am Andrew H. and am a Developer Advocate for Nginx. Part of my
job is to help the community writing such modules and aiding
communication between the community and Nginx’s internal teams.

It is worth noting that at some point in the 1.9.x mainline release we
will be adding dynamic modules which will let you compile modules
out-of-tree and load them on starting Nginx. I’ll reply as best I can
inline.

On 17 Jun 2015, at 12:25, Senko R. [email protected] wrote:

Hi,

I’m writing a new module (out-of-tree) for supporting authentication
using Stormpath’s user management API (https://stormpath.com/).

Basically, the module makes one or more HTTP requests to the
Stormpath API to determine if the client request should be authorized
to access a location or not.

Excellent

Since this is somewhat different than other modules I could learn from, and
since all my knowledge about nginx internals is from looking at how other
modules & core is written, I’m wondering if anyone could comment on how I
designed the module and raise any issues if I did anything problematic,
wrong or weird.

I have scanned through some of the module so far and there doesn’t
appear to be anything wrong or weird.

I haven’t seen any other module do that, but I don’t see that
it’s possible to avoid users having to define upstream manually otherwise.

I agree I have not seen anyone else do this so far. That isn’t to say it
is a bad thing. It is an interesting take on it.

(originally the init is done in ngx_http_upstream_init_main_conf).

github.com

stormpath/stormpath-nginx-module/blob/master/src/ngx_http_auth_stormpath_module.c#L304


      
          static ngx_int_t
          ngx_http_auth_stormpath_process_status_line(ngx_http_request_t *r)
          {
              ngx_int_t         rc;
              ngx_http_status_t status;
          
          
    ngx_memzero(&status, sizeof(ngx_http_status_t));
          
          
    rc = ngx_http_parse_status_line(r, &r->upstream->buffer, &status);
              if (rc == NGX_AGAIN) {
                  return rc;
              }
          
          
    if (rc == NGX_OK) {
                  r->upstream->headers_in.status_n = status.code;
                  r->upstream->process_header = ngx_http_auth_stormpath_process_header;
                  return ngx_http_auth_stormpath_process_header(r);
              }
          
          
    return NGX_HTTP_UPSTREAM_INVALID_HEADER;
          }

This appears to be the correct way to do it as far as I can see, but I’m
happy to defer to the main Nginx developers on this.

(I’ll also hit similar problems with caching the requests to the upstream.
I’d like to reuse the caching functionality already in nginx, but it seems
to me like http_proxy_module does a lot of manual heavy lifting in that
regard that I’d have to reimplement (or shudder copy-paste) to
support it?)

Does the above make sense? Is there an obvious way to do it differently that
I’ve missed? Are there any guides or documentation on how this should be
done (besides Evan M.'s obsolete-but-useful guides I went through
already)?

At the moment there isn’t much beyond Evan M.'s guides and the Nginx
Wiki. I will be creating more up-to-date documentation as we head
towards the dynamic modules feature being released.

Any comments, suggestions, warnings or flames are welcome.

If you have any questions feel free to contact me directly.

Kind Regards

Andrew H. (LinuxJedi)
Senior Developer Advocate
Nginx Inc.