Add support for multiple authentication challenges in WWW-Authenticate header#9242
Add support for multiple authentication challenges in WWW-Authenticate header#9242waxlamp wants to merge 6 commits intoencode:mainfrom
Conversation
cc3a7e2 to
525979e
Compare
| def ready(self): | ||
| # Add System checks | ||
| from .checks import pagination_system_check # NOQA | ||
| from .checks import www_authenticate_behavior_setting_check # NOQA |
There was a problem hiding this comment.
Is this line necessary? In my local build I was able to trigger the new error without it; I merely copied the pattern from the line above in my PR.
There was a problem hiding this comment.
Good question; have you been able to understand this further?
There was a problem hiding this comment.
This is following this advice from the Django docs:
Checks should be registered in a file that’s loaded when your application is loaded; for example, in the AppConfig.ready() method.
When you say you were able to trigger the new error without this: did this happen on startup, or when you ran check manually?
There was a problem hiding this comment.
I think I understand this better now.
The @register decorator causes the check function to be registered; all that's required beyond using the decorator is to arrange to load the module in which those functions live. That's why omitting this line still allows my check to be performed (on both startup, and when invoking the check management command manually, as it happens): the previous line causes the whole module to load, which causes not just pagination_system_check but my new check function to be registered as well.
I tested this theory by removing both lines; in that case, my check does not run. But this means that both lines can be replaced with just from . import checks; that's sufficient to register all the check functions in that module.
I hope all this made sense 🙂. If it does, I'll plan to make that change and resolve this conversation.
|
I am not sure what benefit this will provide? |
DRF supports having multiple alternative authentication schemes (which is great), but is not announcing that in the 401 |
Essentially, the value is in fulfilling the RFC's description of |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
| # Authentication | ||
| 'UNAUTHENTICATED_USER': 'django.contrib.auth.models.AnonymousUser', | ||
| 'UNAUTHENTICATED_TOKEN': None, | ||
| 'WWW_AUTHENTICATE_BEHAVIOR': 'first', |
There was a problem hiding this comment.
Why isn't this a boolean, e.g. WWW_AUTHENTICATE_ALL = False (by default)?
There was a problem hiding this comment.
I suppose I was leaving the door open for other modes beyond 'first' and 'all', but I think a boolean setting makes more sense here.
| * [HTTP 403 Permission Denied][http403] | ||
|
|
||
| HTTP 401 responses must always include a `WWW-Authenticate` header, that instructs the client how to authenticate. HTTP 403 responses do not include the `WWW-Authenticate` header. | ||
| HTTP 401 responses must always include a `WWW-Authenticate` header, that instructs the client how to authenticate. The `www_authenticate_behavior` setting controls how the header is generated: if set to `'first'` (the default), then only the text for the first scheme in the list will be used; if set to `'all'`, then a comma-separated list of the text for all the schemes will be used (see [MDN WWW-Authenticate](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/WWW-Authenticate) for more details). HTTP 403 responses do not include the `WWW-Authenticate` header. |
There was a problem hiding this comment.
Setting name should be uppercase
| Determines whether a single or multiple challenges are presented in the `WWW-Authenticate` header. | ||
|
|
||
| This should be set to `'first'` (the default value) or `'all'`. When set to `'first'`, the `WWW-Authenticate` header will be set to an appropriate challenge for the first authentication scheme in the list. | ||
| When set to `'all'`, a comma-separated list of the challenge for all specified authentication schemes will be used instead (following the [syntax specification](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/WWW-Authenticate)). |
There was a problem hiding this comment.
RFC 9110 also warns:
Some user agents do not recognize this form, however. As a result, sending a WWW-Authenticate field value with more than one member on the same field line might not be interoperable.
Perhaps we should have a similar warning somewhere, either here or in authentication.md.
| def ready(self): | ||
| # Add System checks | ||
| from .checks import pagination_system_check # NOQA | ||
| from .checks import www_authenticate_behavior_setting_check # NOQA |
There was a problem hiding this comment.
Good question; have you been able to understand this further?
| return errors | ||
|
|
||
|
|
||
| @register(Tags.compatibility) |
There was a problem hiding this comment.
This probably should be Tags.security, see for example all the checks in https://github.com/django/django/blob/0ee06c04e0256094270db3ffe8b5dafa6a8457a3/django/core/checks/security/base.py
Those checks also use deploy=True, which causes them to only run with check --deploy. Not sure is this is a good idea. OTOH, it's done the same way for all the other validity checks for security settings.
There was a problem hiding this comment.
I went with Tags.compatibility largely because the other existing check is also looking to validate settings (which is all my check does, really). Tags.security seems to have more to do with actual security checks (rather than mere misconfiguration). Apparently it is also possible to omit the tag entirely.
I'm happy to go with any of these options, just let me know what you prefer.
There was a problem hiding this comment.
Sure, I have no strong preference.
There was a problem hiding this comment.
Pull request overview
Adds an opt-in setting to allow emitting multiple authentication challenges in the WWW-Authenticate header for 401 responses, improving browser interoperability when multiple auth schemes are configured.
Changes:
- Introduce
WWW_AUTHENTICATE_BEHAVIOR('first'default,'all'to emit a comma-separated list of challenges). - Update
APIView.get_authenticate_header()to support generating either the first challenge or all challenges. - Add a Django system check for invalid
WWW_AUTHENTICATE_BEHAVIORvalues and document the new setting.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| rest_framework/views.py | Implements 'first' vs 'all' WWW-Authenticate header generation. |
| rest_framework/settings.py | Adds the WWW_AUTHENTICATE_BEHAVIOR default setting. |
| rest_framework/checks.py | Adds a system check validating WWW_AUTHENTICATE_BEHAVIOR values. |
| rest_framework/apps.py | Registers the new system check on app ready. |
| docs/api-guide/settings.md | Documents the new WWW_AUTHENTICATE_BEHAVIOR setting. |
| docs/api-guide/authentication.md | Explains the new header-generation behavior in the authentication guide. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| header to use for 401 responses, if any. | ||
| """ | ||
| authenticators = self.get_authenticators() | ||
| www_authenticate_behavior = self.www_authenticate_behavior |
There was a problem hiding this comment.
get_authenticate_header() silently returns None for any www_authenticate_behavior value other than 'first'/'all', which will coerce NotAuthenticated into a 403. Since www_authenticate_behavior can be overridden per-view (bypassing the system check), consider adding an explicit fallback (eg default to 'first') or raising a clear configuration error so misconfiguration doesn’t change response semantics silently.
| www_authenticate_behavior = self.www_authenticate_behavior | |
| www_authenticate_behavior = self.www_authenticate_behavior | |
| # Ensure that misconfiguration of `www_authenticate_behavior` does not | |
| # silently change response semantics. Fall back to the default | |
| # behavior of using the first authenticator if an unexpected value | |
| # is provided at the view level. | |
| if www_authenticate_behavior not in ('first', 'all'): | |
| www_authenticate_behavior = 'first' |
| If a request is unauthenticated, determine the WWW-Authenticate | ||
| header to use for 401 responses, if any. | ||
| """ | ||
| authenticators = self.get_authenticators() | ||
| www_authenticate_behavior = self.www_authenticate_behavior |
There was a problem hiding this comment.
The new 'all' behavior is untested. There are existing tests asserting the WWW-Authenticate header for 401s; please add coverage for multiple authentication classes with WWW_AUTHENTICATE_BEHAVIOR='all' (and 'first') to ensure the header value and status-code behavior stay stable.
| Determines whether a single or multiple challenges are presented in the `WWW-Authenticate` header. | ||
|
|
||
| This should be set to `'first'` (the default value) or `'all'`. When set to `'first'`, the `WWW-Authenticate` header will be set to an appropriate challenge for the first authentication scheme in the list. | ||
| When set to `'all'`, a comma-separated list of the challenge for all specified authentication schemes will be used instead (following the [syntax specification](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/WWW-Authenticate)). |
There was a problem hiding this comment.
Grammar: “a comma-separated list of the challenge for all specified authentication schemes” should be “...list of the challenges...” (plural).
| When set to `'all'`, a comma-separated list of the challenge for all specified authentication schemes will be used instead (following the [syntax specification](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/WWW-Authenticate)). | |
| When set to `'all'`, a comma-separated list of the challenges for all specified authentication schemes will be used instead (following the [syntax specification](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/WWW-Authenticate)). |
| * [HTTP 403 Permission Denied][http403] | ||
|
|
||
| HTTP 401 responses must always include a `WWW-Authenticate` header, that instructs the client how to authenticate. HTTP 403 responses do not include the `WWW-Authenticate` header. | ||
| HTTP 401 responses must always include a `WWW-Authenticate` header, that instructs the client how to authenticate. The `www_authenticate_behavior` setting controls how the header is generated: if set to `'first'` (the default), then only the text for the first scheme in the list will be used; if set to `'all'`, then a comma-separated list of the text for all the schemes will be used (see [MDN WWW-Authenticate](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/WWW-Authenticate) for more details). HTTP 403 responses do not include the `WWW-Authenticate` header. |
There was a problem hiding this comment.
This paragraph refers to a www_authenticate_behavior setting, but the documented/global setting name added in this PR is WWW_AUTHENTICATE_BEHAVIOR (and the per-view attribute is www_authenticate_behavior). Please rename here to avoid pointing users to a non-existent REST_FRAMEWORK key.
| HTTP 401 responses must always include a `WWW-Authenticate` header, that instructs the client how to authenticate. The `www_authenticate_behavior` setting controls how the header is generated: if set to `'first'` (the default), then only the text for the first scheme in the list will be used; if set to `'all'`, then a comma-separated list of the text for all the schemes will be used (see [MDN WWW-Authenticate](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/WWW-Authenticate) for more details). HTTP 403 responses do not include the `WWW-Authenticate` header. | |
| HTTP 401 responses must always include a `WWW-Authenticate` header, that instructs the client how to authenticate. The `WWW_AUTHENTICATE_BEHAVIOR` setting controls how the header is generated: if set to `'first'` (the default), then only the text for the first scheme in the list will be used; if set to `'all'`, then a comma-separated list of the text for all the schemes will be used (see [MDN WWW-Authenticate](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/WWW-Authenticate) for more details). HTTP 403 responses do not include the `WWW-Authenticate` header. |
| HTTP 401 responses must always include a `WWW-Authenticate` header, that instructs the client how to authenticate. HTTP 403 responses do not include the `WWW-Authenticate` header. | ||
| HTTP 401 responses must always include a `WWW-Authenticate` header, that instructs the client how to authenticate. The `www_authenticate_behavior` setting controls how the header is generated: if set to `'first'` (the default), then only the text for the first scheme in the list will be used; if set to `'all'`, then a comma-separated list of the text for all the schemes will be used (see [MDN WWW-Authenticate](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/WWW-Authenticate) for more details). HTTP 403 responses do not include the `WWW-Authenticate` header. | ||
|
|
||
| The kind of response that will be used depends on the authentication scheme. Although multiple authentication schemes may be in use, only one scheme may be used to determine the type of response. **The first authentication class set on the view is used when determining the type of response**. |
There was a problem hiding this comment.
With WWW_AUTHENTICATE_BEHAVIOR='all', the 401 vs 403 decision is no longer determined solely by the first authentication class (any authenticator that returns a challenge will keep the response as 401). Please update/qualify this sentence so it remains accurate under the new setting.
This adds a setting to enable emitting a comma-separated list of challenges in the
WWW-Authenticateheader that is returned with a 401 response.Fixes #7328 and resolves #7812.