Conversation
This comment has been minimized.
This comment has been minimized.
✅ Docs preview readyThe preview is ready to be viewed. View the preview File Changes 0 new, 1 changed, 0 removedBuild ID: b16fd17b47dd1e584f7bc824 URL: https://www.apollographql.com/docs/deploy-preview/b16fd17b47dd1e584f7bc824 |
| name: super::default_header_name(), | ||
| value_prefix: super::default_header_value_prefix(), | ||
| }); | ||
| match authenticate(&config, &manager, request.try_into().unwrap()) { |
There was a problem hiding this comment.
Is this testing that a JWT with no audience claim is always accepted? Should we accept or reject such claims?
There was a problem hiding this comment.
I don't know what's more correct. I took the behavior from what we're doing with iss already. I figure we can accept them now and reject in the future if we learn it's a bad thing to do.
There was a problem hiding this comment.
I'm starting to think that the iss and aud should both reject if the issuers or audiences options are configured, but the value in the token is missing or not a string...
There was a problem hiding this comment.
I made changes to this so that we'll reject when client aud is missing if server one is configured.
There was a problem hiding this comment.
I'm not going to do it for issuer since I don't want to break anyone.
goto-bus-stop
left a comment
goto-bus-stop
left a comment
There was a problem hiding this comment.
Looks good, does need a changeset!
3 participants
Exactly what it says on the tin.
Checklist
Complete the checklist (and note appropriate exceptions) before the PR is marked ready-for-review.
Exceptions
Note any exceptions here
Notes
Footnotes
It may be appropriate to bring upcoming changes to the attention of other (impacted) groups. Please endeavour to do this before seeking PR approval. The mechanism for doing this will vary considerably, so use your judgement as to how and when to do this. ↩
Configuration is an important part of many changes. Where applicable please try to document configuration examples. ↩
A lot of (if not most) features benefit from built-in observability and
debug-level logs. Please read this guidance on metrics best-practices. ↩Tick whichever testing boxes are applicable. If you are adding Manual Tests, please document the manual testing (extensively) in the Exceptions. ↩