Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

authenticate: add jwks and .well-known endpoint #745

Merged
merged 4 commits into from May 21, 2020

Conversation

desimone
Copy link
Contributor

@desimone desimone commented May 21, 2020

Summary

Screen Shot 2020-05-20 at 9 09 03 PM

Screen Shot 2020-05-20 at 9 08 54 PM

Related issues

See also

Checklist:

  • add related issues
  • updated docs (TODO!)
  • updated unit tests
  • updated UPGRADING.md
  • ready for review

@desimone desimone added this to the 0.9.0 milestone May 21, 2020
@desimone desimone requested a review from a team as a code owner May 21, 2020 04:13
@desimone desimone requested a review from travisgroth May 21, 2020 04:13
@codecov
Copy link

codecov bot commented May 21, 2020

Codecov Report

Merging #745 into master will decrease coverage by 0.5%.
The diff coverage is 51.8%.

@@           Coverage Diff            @@
##           master    #745     +/-   ##
========================================
- Coverage    75.0%   74.4%   -0.6%     
========================================
  Files          58      59      +1     
  Lines        3154    3230     +76     
========================================
+ Hits         2366    2404     +38     
- Misses        638     670     +32     
- Partials      150     156      +6     
Impacted Files Coverage Δ
authenticate/handlers.go 77.8% <13.3%> (-6.8%) ⬇️
authenticate/authenticate.go 93.1% <66.6%> (-4.3%) ⬇️
internal/cryptutil/jose.go 78.3% <78.3%> (ø)

Signed-off-by: Bobby DeSimone <bobbydesimone@gmail.com>
Signed-off-by: Bobby DeSimone <bobbydesimone@gmail.com>
Signed-off-by: Bobby DeSimone <bobbydesimone@gmail.com>
Copy link
Contributor

@calebdoxsey calebdoxsey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few small typos, but LGTM.

wellKnownURLS := struct {
// URL string referencing the client's JSON Web Key (JWK) Set
// RFC7517 document, which contains the client's public keys.
JSONWebWeySetURL string `json:"jwks_uri"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: JSONWebWeySetURL should be JSONWebKeySetURL


If no certificate is specified, one will be generated for you and the base64'd public key will be added to the logs.
If no certificate is specified, one will be generated and the base64'd public key will be added to the logs. Note, hoever, that this key be unique to each service, ephemerial, and will not be accessible via the authenticate service's `jwks_uri` endpoint.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: hoever should be however, ephemerial should be ephemeral

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@calebdoxsey I'm having one of those days...

Signed-off-by: Bobby DeSimone <bobbydesimone@gmail.com>
@desimone desimone merged commit 3f1faf2 into pomerium:master May 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expose (public) Signing Key for JWT
2 participants