Opened 12 years ago
Closed 11 years ago
#285 closed defect (fixed)
When "satisfy any" is set the unauth access_code (401) should not be overriden by forbidden (403) access_code
| Reported by: | Jan Marc Hoffmann | Owned by: | |
|---|---|---|---|
| Priority: | major | Milestone: | |
| Component: | nginx-core | Version: | 1.3.x |
| Keywords: | satisfy access | Cc: | |
| uname -a: | Linux cache 2.6.32-17-pve #1 SMP Tue Jan 8 12:40:35 UTC 2013 x86_64 Intel(R) Core(TM) i7-3770S CPU @ 3.10GHz GenuineIntel GNU/Linux | ||
| nginx -V: |
nginx version: nginx/1.3.11
TLS SNI support enabled configure arguments: --prefix=/usr --sbin-path=/usr/sbin/nginx --conf-path=/etc/nginx/nginx.conf --error-log-path=/var/log/nginx/error_log --pid-path=/var/run/nginx.pid --lock-path=/var/lock/nginx.lock --user=nginx --group=nginx --with-cc-opt=-I/usr/include --with-ld-opt=-L/usr/lib --http-log-path=/var/log/nginx/access_log --http-client-body-temp-path=/var/tmp/nginx/client --http-proxy-temp-path=/var/tmp/nginx/proxy --http-fastcgi-temp-path=/var/tmp/nginx/fastcgi --http-scgi-temp-path=/var/tmp/nginx/scgi --http-uwsgi-temp-path=/var/tmp/nginx/uwsgi --with-debug --with-pcre --with-http_sub_module --with-http_realip_module --add-module=/var/tmp/portage/www-servers/nginx-1.3.11/work/ngx_pagespeed-master --add-module=/var/tmp/portage/www-servers/nginx-1.3.11/work/nginx-auth-ldap-master --with-http_ssl_module --without-mail_imap_module --without-mail_pop3_module --without-mail_smtp_module |
||
Description
Current behaviour:
Two access modules are registered. Module1 returns NGX_HTTP_FORBIDDEN and module2 returns NGX_HTTP_UNAUTHORIZED on the first request. Nginx returns the code of the first module loaded. So the first request will return NGX_HTTP_FORBIDDEN.
Problem:
This does not go along with the documentation which states that any module can grant access. After returning 403 the browser will not send Basic Authentication. So module2 never gets a chance to grant access.
Expected:
If "satisfy any" is selected modules will not overwrite unauth with forbidden.
I have attached a patch which fixes this problem.
Attachments (2)
Change History (8)
by , 12 years ago
| Attachment: | satisfy_any_unauth_forbidden.patch added |
|---|
comment:1 by , 12 years ago
| Status: | new → accepted |
|---|
comment:2 by , 12 years ago
Hi Maxim,
which check is always false?
A = clcf->satisfy == NGX_HTTP_SATISFY_ALL (true on satisfy all, false on satisfy any)
B = r->access_code != NGX_HTTP_UNAUTHORIZED (true on forbidden, false on unauth)
A, B, A or B
true true true
true false true
false true true
false false false
So its only false on satisfy any and unauth, which is correct since in this case it should not get overriden.
comment:3 by , 12 years ago
The code is in else part of the following if():
if (clcf->satisfy == NGX_HTTP_SATISFY_ALL) {
...
} else {
...
}
Hence clcf->satisfy can't be NGX_HTTP_SATISFY_ALL there. And it's logical, as with NGX_HTTP_SATISFY_ALL the error is immediately returned.
comment:4 by , 12 years ago
Right. It's a dead check so it can get safely removed. I changed the patch.
by , 12 years ago
| Attachment: | satisfy_any_unauth_forbidden.2.patch added |
|---|
Content-Disposition: form-data; name="replace"
on
The patch looks wrong as it contains check which is always false, but in general change to prefer 401 over 403 looks right. As of now there is no problem with official nginx modules due to access module being checked before auth basic, but it would be clearly beneficial for 3rd party modules.