-
Notifications
You must be signed in to change notification settings - Fork 0
Split session #25
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
Merged
Merged
Split session #25
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
dff7459
Session -> Verify+Session
klaidliadon d2becfb
fix name
klaidliadon 1a244ab
remove project store
klaidliadon e47298f
Project JWT verify
klaidliadon 71f223d
wrap error
klaidliadon 624549f
CR suggestions
klaidliadon ad88344
Don't pass request
klaidliadon d1a1668
more CR suggestions
klaidliadon 6c0ffd9
Use assert
klaidliadon bd92776
Simplify by removing abstraction
klaidliadon debadf2
minor changes
klaidliadon 30ad26a
remove constant
klaidliadon 8b89854
add explanations
klaidliadon 339ef10
rename service
klaidliadon File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,18 +16,22 @@ import ( | |
|
|
||
| // Options for the authcontrol middleware handlers Session and AccessControl. | ||
| type Options struct { | ||
| // JWT secret used to verify the JWT token. | ||
| // JWTsecret is required, and it is used for the JWT verification. | ||
| // If a Project Store is also provided and the request has a project claim, | ||
| // it could be replaced by the a specific verifier. | ||
| JWTSecret string | ||
|
|
||
| // ProjectStore is a pluggable backends that verifies if the project from the claim exists. | ||
| // When provived, it checks the Project from the JWT, and can override the JWT Auth. | ||
| ProjectStore ProjectStore | ||
|
|
||
| // AccessKeyFuncs are used to extract the access key from the request. | ||
| AccessKeyFuncs []AccessKeyFunc | ||
|
|
||
| // UserStore is a pluggable backends that verifies if the account exists. | ||
| // When provided, it can upgrade a Wallet session to a User or Admin session. | ||
| UserStore UserStore | ||
|
|
||
| // ProjectStore is a pluggable backends that verifies if the project exists. | ||
| ProjectStore ProjectStore | ||
|
|
||
| // ErrHandler is a function that is used to handle and respond to errors. | ||
| ErrHandler ErrHandler | ||
| } | ||
|
|
@@ -46,34 +50,47 @@ func (o *Options) ApplyDefaults() { | |
| } | ||
| } | ||
|
|
||
| func Session(cfg Options) func(next http.Handler) http.Handler { | ||
| func VerifyToken(cfg Options) func(next http.Handler) http.Handler { | ||
| cfg.ApplyDefaults() | ||
| auth := jwtauth.New("HS256", []byte(cfg.JWTSecret), nil, jwt.WithAcceptableSkew(2*time.Minute)) | ||
| jwtOptions := []jwt.ValidateOption{ | ||
| jwt.WithAcceptableSkew(2 * time.Minute), | ||
| } | ||
|
|
||
| return func(next http.Handler) http.Handler { | ||
| return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| ctx := r.Context() | ||
|
|
||
| // check if the request already contains session, if it does then continue | ||
| if _, ok := GetSessionType(ctx); ok { | ||
| next.ServeHTTP(w, r) | ||
| return | ||
| } | ||
| auth := NewAuth(cfg.JWTSecret) | ||
|
|
||
| var ( | ||
| sessionType proto.SessionType | ||
| accessKey string | ||
| token jwt.Token | ||
| ) | ||
| if cfg.ProjectStore != nil { | ||
| projectID, err := findProjectClaim(r) | ||
| if err != nil { | ||
| cfg.ErrHandler(r, w, proto.ErrUnauthorized.WithCausef("get project claim: %w", err)) | ||
| return | ||
| } | ||
|
|
||
| for _, f := range cfg.AccessKeyFuncs { | ||
| if accessKey = f(r); accessKey != "" { | ||
| break | ||
| project, _auth, err := cfg.ProjectStore.GetProject(ctx, projectID) | ||
| if err != nil { | ||
| cfg.ErrHandler(r, w, proto.ErrUnauthorized.WithCausef("get project: %w", err)) | ||
| return | ||
| } | ||
| if project == nil { | ||
| cfg.ErrHandler(r, w, proto.ErrProjectNotFound) | ||
| return | ||
| } | ||
| if _auth != nil { | ||
| auth = _auth | ||
| } | ||
| ctx = WithProject(ctx, project) | ||
| } | ||
|
|
||
| // Verify JWT token and validate its claims. | ||
| token, err := jwtauth.VerifyRequest(auth, r, jwtauth.TokenFromHeader) | ||
| jwtAuth, err := auth.GetVerifier(jwtOptions...) | ||
| if err != nil { | ||
| cfg.ErrHandler(r, w, proto.ErrUnauthorized.WithCausef("get verifier: %w", err)) | ||
| return | ||
| } | ||
|
|
||
| token, err := jwtauth.VerifyRequest(jwtAuth, r, jwtauth.TokenFromHeader) | ||
| if err != nil { | ||
| if errors.Is(err, jwtauth.ErrExpired) { | ||
| cfg.ErrHandler(r, w, proto.ErrSessionExpired) | ||
|
|
@@ -89,7 +106,7 @@ func Session(cfg Options) func(next http.Handler) http.Handler { | |
| if token != nil { | ||
| claims, err := token.AsMap(ctx) | ||
| if err != nil { | ||
| cfg.ErrHandler(r, w, err) | ||
| cfg.ErrHandler(r, w, proto.ErrUnauthorized.WithCausef("invalid token: %w", err)) | ||
| return | ||
| } | ||
|
|
||
|
|
@@ -102,6 +119,44 @@ func Session(cfg Options) func(next http.Handler) http.Handler { | |
| } | ||
| } | ||
|
|
||
| ctx = jwtauth.NewContext(ctx, token, nil) | ||
| } | ||
|
|
||
| next.ServeHTTP(w, r.WithContext(ctx)) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func Session(cfg Options) func(next http.Handler) http.Handler { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Session needs ErrorHandler, UserStore and a way to fetch AccessKey. |
||
| cfg.ApplyDefaults() | ||
|
|
||
| return func(next http.Handler) http.Handler { | ||
| return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| ctx := r.Context() | ||
|
|
||
| // if a custom middleware already sets the session type, skip this middleware | ||
| if _, ok := GetSessionType(ctx); ok { | ||
| next.ServeHTTP(w, r) | ||
| return | ||
| } | ||
|
|
||
| var ( | ||
| accessKey string | ||
| sessionType proto.SessionType | ||
| ) | ||
|
|
||
| for _, f := range cfg.AccessKeyFuncs { | ||
| if accessKey = f(r); accessKey != "" { | ||
| break | ||
| } | ||
| } | ||
|
|
||
| _, claims, err := jwtauth.FromContext(ctx) | ||
| if err != nil { | ||
| cfg.ErrHandler(r, w, err) | ||
| return | ||
| } | ||
| if claims != nil { | ||
| serviceClaim, _ := claims["service"].(string) | ||
| accountClaim, _ := claims["account"].(string) | ||
| adminClaim, _ := claims["admin"].(bool) | ||
|
|
@@ -140,20 +195,7 @@ func Session(cfg Options) func(next http.Handler) http.Handler { | |
| } | ||
|
|
||
| if projectClaim > 0 { | ||
| projectID := uint64(projectClaim) | ||
| if cfg.ProjectStore != nil { | ||
| project, err := cfg.ProjectStore.GetProject(ctx, projectID) | ||
| if err != nil { | ||
| cfg.ErrHandler(r, w, err) | ||
| return | ||
| } | ||
| if project == nil { | ||
| cfg.ErrHandler(r, w, proto.ErrProjectNotFound) | ||
| return | ||
| } | ||
| ctx = WithProject(ctx, project) | ||
| } | ||
| ctx = WithProjectID(ctx, projectID) | ||
| ctx = WithProjectID(ctx, uint64(projectClaim)) | ||
| sessionType = proto.SessionType_Project | ||
| } | ||
| } | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like we could split Options struct into two types -- one for Verifier and one for Sessions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Imho, this middleware needs nothing but
JWTSecretandErrHandler. It should always verify HS256 JWT from both Authorization header and Cookie.I don't see the need for
AuthProviderinterface. Is it useful for anything else? I think it just locks us down to this implementation -- switching algorithms, jwks or supporting secret rotation or multiple algorithms would be more difficult.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Imho, stack/api's
ProjectJWTVerifiershould be a a separate implementation that doesn't have anything in common with this Verifier except for passing the token into context viactx = jwtauth.NewContext(ctx, token, nil)