From 5dd5d7a0801df83fb88739d3ccfe87bfc9011975 Mon Sep 17 00:00:00 2001 From: Stefan Sakalik Date: Thu, 24 Aug 2017 18:01:24 +0100 Subject: [PATCH 1/4] Add support for claim groups in keycloak-proxy. --- middleware.go | 94 +++++++++++++++++++++++++++++++++------------------ 1 file changed, 61 insertions(+), 33 deletions(-) diff --git a/middleware.go b/middleware.go index d7bc79108..f1f8cf9b0 100644 --- a/middleware.go +++ b/middleware.go @@ -228,6 +228,65 @@ func (r *oauthProxy) authenticationMiddleware(resource *Resource) func(http.Hand } } +// checkClaim checks whether claim in userContext matches claimName, match. It can be String or Strings claim. +func (r *oauthProxy) checkClaim(user *userContext, claimName string, match *regexp.Regexp, resourceURL string) bool { + // Check string claim. + valueStr, foundStr, errStr := user.claims.StringClaim(claimName) + // We have found string claim, so let's check whether it matches. + if foundStr { + if match.MatchString(valueStr) { + return true + } + r.log.Warn("claim requirement does not match claim in token", + zap.String("access", "denied"), + zap.String("claim", claimName), + zap.String("email", user.email), + zap.String("issued", valueStr), + zap.String("required", match.String()), + zap.String("resource", resourceURL)) + + return false + } + + // Check strings claim. + valueStrs, foundStrs, errStrs := user.claims.StringsClaim(claimName) + // We have found strings claim, so let's check whether it matches. + if foundStrs { + for _, value := range valueStrs { + if match.MatchString(value) { + return true + } + } + r.log.Warn("claim requirement does not match any element claim group in token", + zap.String("access", "denied"), + zap.String("claim", claimName), + zap.String("email", user.email), + zap.String("issued", fmt.Sprintf("%v", valueStrs)), + zap.String("required", match.String()), + zap.String("resource", resourceURL)) + + return false + } + + // If this fails, the claim is probably float or int. + if errStr != nil && errStrs != nil { + r.log.Error("unable to extract the claim from token (tried string and strings)", + zap.String("access", "denied"), + zap.String("email", user.email), + zap.String("resource", resourceURL), + zap.Error(errStr), + zap.Error(errStrs)) + return false + } + + r.log.Warn("the token does not have the claim", + zap.String("access", "denied"), + zap.String("claim", claimName), + zap.String("email", user.email), + zap.String("resource", resourceURL)) + return false +} + // admissionMiddleware is responsible checking the access token against the protected resource func (r *oauthProxy) admissionMiddleware(resource *Resource) func(http.Handler) http.Handler { claimMatches := make(map[string]*regexp.Regexp) @@ -261,39 +320,8 @@ func (r *oauthProxy) admissionMiddleware(resource *Resource) func(http.Handler) // step: if we have any claim matching, lets validate the tokens has the claims for claimName, match := range claimMatches { - value, found, err := user.claims.StringClaim(claimName) - if err != nil { - r.log.Error("unable to extract the claim from token", - zap.String("access", "denied"), - zap.String("email", user.email), - zap.String("resource", resource.URL), - zap.Error(err)) - - next.ServeHTTP(w, req.WithContext(r.accessForbidden(w, req))) - return - } - - if !found { - r.log.Warn("the token does not have the claim", - zap.String("access", "denied"), - zap.String("claim", claimName), - zap.String("email", user.email), - zap.String("resource", resource.URL)) - - next.ServeHTTP(w, req.WithContext(r.accessForbidden(w, req))) - return - } - - // step: check the claim is the same - if !match.MatchString(value) { - r.log.Warn("the token claims does not match claim requirement", - zap.String("access", "denied"), - zap.String("claim", claimName), - zap.String("email", user.email), - zap.String("issued", value), - zap.String("required", match.String()), - zap.String("resource", resource.URL)) - + // TODO: handle errors. + if !r.checkClaim(user, claimName, match, resource.URL) { next.ServeHTTP(w, req.WithContext(r.accessForbidden(w, req))) return } From eeb9af53ea5aaca88cb98c889da76a9da5a73bd1 Mon Sep 17 00:00:00 2001 From: Stefan Sakalik Date: Thu, 24 May 2018 16:16:23 +0100 Subject: [PATCH 2/4] Fixes + simplifications. --- middleware.go | 37 +++++++++++++++------------------- middleware_test.go | 49 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 21 deletions(-) diff --git a/middleware.go b/middleware.go index f1f8cf9b0..7a77618e7 100644 --- a/middleware.go +++ b/middleware.go @@ -29,6 +29,7 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/unrolled/secure" "go.uber.org/zap" + "go.uber.org/zap/zapcore" ) const ( @@ -230,6 +231,13 @@ func (r *oauthProxy) authenticationMiddleware(resource *Resource) func(http.Hand // checkClaim checks whether claim in userContext matches claimName, match. It can be String or Strings claim. func (r *oauthProxy) checkClaim(user *userContext, claimName string, match *regexp.Regexp, resourceURL string) bool { + errFields := []zapcore.Field{ + zap.String("claim", claimName), + zap.String("access", "denied"), + zap.String("email", user.email), + zap.String("resource", resourceURL), + } + // Check string claim. valueStr, foundStr, errStr := user.claims.StringClaim(claimName) // We have found string claim, so let's check whether it matches. @@ -237,13 +245,10 @@ func (r *oauthProxy) checkClaim(user *userContext, claimName string, match *rege if match.MatchString(valueStr) { return true } - r.log.Warn("claim requirement does not match claim in token", - zap.String("access", "denied"), - zap.String("claim", claimName), - zap.String("email", user.email), + r.log.Warn("claim requirement does not match claim in token", append(errFields, zap.String("issued", valueStr), zap.String("required", match.String()), - zap.String("resource", resourceURL)) + )...) return false } @@ -257,33 +262,24 @@ func (r *oauthProxy) checkClaim(user *userContext, claimName string, match *rege return true } } - r.log.Warn("claim requirement does not match any element claim group in token", - zap.String("access", "denied"), - zap.String("claim", claimName), - zap.String("email", user.email), + r.log.Warn("claim requirement does not match any element claim group in token", append(errFields, zap.String("issued", fmt.Sprintf("%v", valueStrs)), zap.String("required", match.String()), - zap.String("resource", resourceURL)) + )...) return false } // If this fails, the claim is probably float or int. if errStr != nil && errStrs != nil { - r.log.Error("unable to extract the claim from token (tried string and strings)", - zap.String("access", "denied"), - zap.String("email", user.email), - zap.String("resource", resourceURL), + r.log.Error("unable to extract the claim from token (tried string and strings)", append(errFields, zap.Error(errStr), - zap.Error(errStrs)) + zap.Error(errStrs), + )...) return false } - r.log.Warn("the token does not have the claim", - zap.String("access", "denied"), - zap.String("claim", claimName), - zap.String("email", user.email), - zap.String("resource", resourceURL)) + r.log.Warn("the token does not have the claim", errFields...) return false } @@ -320,7 +316,6 @@ func (r *oauthProxy) admissionMiddleware(resource *Resource) func(http.Handler) // step: if we have any claim matching, lets validate the tokens has the claims for claimName, match := range claimMatches { - // TODO: handle errors. if !r.checkClaim(user, claimName, match, resource.URL) { next.ServeHTTP(w, req.WithContext(r.accessForbidden(w, req))) return diff --git a/middleware_test.go b/middleware_test.go index 13f9f3cb8..f472a4b33 100644 --- a/middleware_test.go +++ b/middleware_test.go @@ -1049,6 +1049,7 @@ func TestRolesAdmissionHandlerClaims(t *testing.T) { Matches map[string]string Request fakeRequest }{ + // jose.StringClaim test { Matches: map[string]string{"cal": "test"}, Request: fakeRequest{ @@ -1126,6 +1127,54 @@ func TestRolesAdmissionHandlerClaims(t *testing.T) { ExpectedCode: http.StatusOK, }, }, + // jose.StringsClaim test + { + Matches: map[string]string{"item": "^t.*t"}, + Request: fakeRequest{ + URI: uri, + HasToken: true, + TokenClaims: jose.Claims{"item": []string{"nonMatchingClaim", "test", "anotherNonMatching"}}, + ExpectedProxy: true, + ExpectedCode: http.StatusOK, + }, + }, + { + Matches: map[string]string{"item": "^t.*t"}, + Request: fakeRequest{ + URI: uri, + HasToken: true, + TokenClaims: jose.Claims{"item": []string{"1test", "2test", "3test"}}, + ExpectedProxy: false, + ExpectedCode: http.StatusForbidden, + }, + }, + { + Matches: map[string]string{"item": "^t.*t"}, + Request: fakeRequest{ + URI: uri, + HasToken: true, + TokenClaims: jose.Claims{"item": []string{}}, + ExpectedProxy: false, + ExpectedCode: http.StatusForbidden, + }, + }, + { + Matches: map[string]string{ + "item1": "^t.*t", + "item2": "^another", + }, + Request: fakeRequest{ + URI: uri, + HasToken: true, + TokenClaims: jose.Claims{ + "item1": []string{"randomItem", "test"}, + "item2": []string{"randomItem", "anotherItem"}, + "item3": []string{"randomItem2", "anotherItem3"}, + }, + ExpectedProxy: true, + ExpectedCode: http.StatusOK, + }, + }, } for _, c := range requests { cfg := newFakeKeycloakConfig() From 96d8452d4224b8ff690bc8d1a851732c02aafdef Mon Sep 17 00:00:00 2001 From: Stefan Sakalik Date: Fri, 25 May 2018 18:09:22 +0100 Subject: [PATCH 3/4] address comment --- README.md | 17 +++++++++++++++++ middleware.go | 7 ++++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 18f60b8e6..f73760b4e 100644 --- a/README.md +++ b/README.md @@ -447,6 +447,23 @@ match-claims: email: ^.*@example.com$ ``` +Proxy supports matching on multivalue Strings claims. In this case the match will succeed if it matches one of the claim values. For example: + +```YAML +match-claims: + perms: perm1 +``` + +will successfully match + +```JSON +{ + "iss": "https://sso.example.com", + "sub": "", + "perms": ["perm1", "perm2"] +} +``` + #### **Groups Claims** You can match on the group claims within a token via the `groups` parameter available within the resource. Note while roles are implicitly required i.e. `roles=admin,user` the user MUST have roles 'admin' AND 'user', groups are applied with an OR operation, so `groups=users,testers` requires the user MUST be within 'users' OR 'testers'. At present the claim name is hardcoded to `groups` i.e a JWT token would look like the below. diff --git a/middleware.go b/middleware.go index c2e1d54e3..6ea0b2932 100644 --- a/middleware.go +++ b/middleware.go @@ -221,6 +221,11 @@ func (r *oauthProxy) checkClaim(user *userContext, claimName string, match *rege zap.String("resource", resourceURL), } + if _, found := user.claims[claimName]; !found { + r.log.Warn("the token does not have the claim", errFields...) + return false + } + // Check string claim. valueStr, foundStr, errStr := user.claims.StringClaim(claimName) // We have found string claim, so let's check whether it matches. @@ -262,7 +267,7 @@ func (r *oauthProxy) checkClaim(user *userContext, claimName string, match *rege return false } - r.log.Warn("the token does not have the claim", errFields...) + r.log.Warn("unexpected error", errFields...) return false } From b77f33a24f7e3a111a3aa8fc61345e5051f8c0a5 Mon Sep 17 00:00:00 2001 From: Stefan Sakalik Date: Fri, 25 May 2018 18:11:40 +0100 Subject: [PATCH 4/4] fix ed --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index f73760b4e..ddb0dfc05 100644 --- a/README.md +++ b/README.md @@ -447,7 +447,7 @@ match-claims: email: ^.*@example.com$ ``` -Proxy supports matching on multivalue Strings claims. In this case the match will succeed if it matches one of the claim values. For example: +The proxy supports matching on multivalue Strings claims. The match will succeed if one of the values matches, for example: ```YAML match-claims: