From 2eecf756e43a2d77e489d41251af4ab2f419aa06 Mon Sep 17 00:00:00 2001 From: Ryan Luckie Date: Fri, 3 May 2019 17:33:56 -0500 Subject: [PATCH 01/19] Add OIDC support for UserInfo Endpoint Email Verification * Current OIDC implementation asserts that user email check must come from JWT token claims. OIDC specification also allows for source of user email to be fetched from userinfo profile endpoint. http://openid.net/specs/openid-connect-core-1_0.html#UserInfo * First, attempt to retrieve email from JWT token claims. Then fall back to requesting email from userinfo endpoint. * Don't fallback to subject for email https://github.com/bitly/oauth2_proxy/pull/481 --- providers/oidc.go | 38 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/providers/oidc.go b/providers/oidc.go index 86a58f6..396310f 100644 --- a/providers/oidc.go +++ b/providers/oidc.go @@ -3,11 +3,15 @@ package providers import ( "context" "fmt" + "net/http" "time" oidc "github.com/coreos/go-oidc" "github.com/pusher/oauth2_proxy/pkg/apis/sessions" + "github.com/pusher/oauth2_proxy/pkg/requests" + "golang.org/x/oauth2" + ) // OIDCProvider represents an OIDC based Identity Provider @@ -117,8 +121,31 @@ func (p *OIDCProvider) createSessionState(ctx context.Context, token *oauth2.Tok } if claims.Email == "" { - // TODO: Try getting email from /userinfo before falling back to Subject - claims.Email = claims.Subject + if p.ProfileURL.String() == "" { + return nil, fmt.Errorf("id_token did not contain an email") + } + + // If the userinfo endpoint profileURL is defined, then there is a chance the userinfo + // contents at the profileURL contains the email. + // Make a query to the userinfo endpoint, and attempt to locate the email from there. + + req, err := http.NewRequest("GET", p.ProfileURL.String(), nil) + if err != nil { + return nil, err + } + req.Header = getOIDCHeader(token.AccessToken) + + json, err := requests.Request(req) + if err != nil { + return nil, err + } + + email, err := json.Get("email").String() + if err != nil { + return nil, fmt.Errorf("id_token nor userinfo endpoint did not contain an email") + } + + claims.Email = email } if !p.AllowUnverifiedEmail && claims.Verified != nil && !*claims.Verified { return nil, fmt.Errorf("email in id_token (%s) isn't verified", claims.Email) @@ -145,3 +172,10 @@ func (p *OIDCProvider) ValidateSessionState(s *sessions.SessionState) bool { return true } + +func getOIDCHeader(access_token string) http.Header { + header := make(http.Header) + header.Set("Accept", "application/json") + header.Set("Authorization", fmt.Sprintf("Bearer %s", access_token)) + return header +} From 0d94f5e51597628a5dc920a723290a73e6d59695 Mon Sep 17 00:00:00 2001 From: Ryan Luckie Date: Fri, 3 May 2019 23:07:48 +0000 Subject: [PATCH 02/19] fix lint error --- providers/oidc.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/providers/oidc.go b/providers/oidc.go index 396310f..937a16b 100644 --- a/providers/oidc.go +++ b/providers/oidc.go @@ -173,9 +173,9 @@ func (p *OIDCProvider) ValidateSessionState(s *sessions.SessionState) bool { return true } -func getOIDCHeader(access_token string) http.Header { +func getOIDCHeader(accessToken string) http.Header { header := make(http.Header) header.Set("Accept", "application/json") - header.Set("Authorization", fmt.Sprintf("Bearer %s", access_token)) + header.Set("Authorization", fmt.Sprintf("Bearer %s", accessToken)) return header } From 122ec45dd8f3da63d74a91fcd31e6ed5e11bdf49 Mon Sep 17 00:00:00 2001 From: Ryan Luckie Date: Tue, 7 May 2019 16:17:38 -0500 Subject: [PATCH 03/19] Requested changes --- providers/oidc.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/providers/oidc.go b/providers/oidc.go index 937a16b..7b3df62 100644 --- a/providers/oidc.go +++ b/providers/oidc.go @@ -11,7 +11,6 @@ import ( "github.com/pusher/oauth2_proxy/pkg/requests" "golang.org/x/oauth2" - ) // OIDCProvider represents an OIDC based Identity Provider @@ -135,12 +134,12 @@ func (p *OIDCProvider) createSessionState(ctx context.Context, token *oauth2.Tok } req.Header = getOIDCHeader(token.AccessToken) - json, err := requests.Request(req) + respJson, err := requests.Request(req) if err != nil { return nil, err } - email, err := json.Get("email").String() + email, err := respJson.Get("email").String() if err != nil { return nil, fmt.Errorf("id_token nor userinfo endpoint did not contain an email") } From f537720b5290e8e9b8d222fe56c77567b98f4746 Mon Sep 17 00:00:00 2001 From: Ryan Luckie Date: Tue, 7 May 2019 16:29:21 -0500 Subject: [PATCH 04/19] fix lint errors --- providers/oidc.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/providers/oidc.go b/providers/oidc.go index 7b3df62..3eaac15 100644 --- a/providers/oidc.go +++ b/providers/oidc.go @@ -134,12 +134,12 @@ func (p *OIDCProvider) createSessionState(ctx context.Context, token *oauth2.Tok } req.Header = getOIDCHeader(token.AccessToken) - respJson, err := requests.Request(req) + respJSON, err := requests.Request(req) if err != nil { return nil, err } - email, err := respJson.Get("email").String() + email, err := respJSON.Get("email").String() if err != nil { return nil, fmt.Errorf("id_token nor userinfo endpoint did not contain an email") } From 93cb575d7c94ea8ec5056c100fd0a4779ffcd8c5 Mon Sep 17 00:00:00 2001 From: Ryan Luckie Date: Fri, 19 Jul 2019 08:59:29 -0500 Subject: [PATCH 05/19] Fix error message for clarity --- providers/oidc.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/providers/oidc.go b/providers/oidc.go index 3eaac15..2abf2ca 100644 --- a/providers/oidc.go +++ b/providers/oidc.go @@ -141,7 +141,7 @@ func (p *OIDCProvider) createSessionState(ctx context.Context, token *oauth2.Tok email, err := respJSON.Get("email").String() if err != nil { - return nil, fmt.Errorf("id_token nor userinfo endpoint did not contain an email") + return nil, fmt.Errorf("Neither id_token nor userinfo endpoint contained an email") } claims.Email = email From 4a6b703c543b72e60421ca597749cd08780701e8 Mon Sep 17 00:00:00 2001 From: Ryan Luckie Date: Fri, 19 Jul 2019 09:03:01 -0500 Subject: [PATCH 06/19] Update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74f09c2..4166882 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -88,6 +88,7 @@ - [#159](https://github.com/pusher/oauth2_proxy/pull/159) Add option to skip the OIDC provider verified email check: `--insecure-oidc-allow-unverified-email` - [#210](https://github.com/pusher/oauth2_proxy/pull/210) Update base image from Alpine 3.9 to 3.10 (@steakunderscore) - [#211](https://github.com/pusher/oauth2_proxy/pull/211) Switch from dep to go modules (@steakunderscore) +- [#145](https://github.com/pusher/oauth2_proxy/pull/145) Add support for OIDC UserInfo endpoint email verification (@rtluckie) # v3.2.0 From e48d28d1b9dd36c53194386c60fb13a2766528e2 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Tue, 23 Jul 2019 16:20:45 +0100 Subject: [PATCH 07/19] Add MAINTAINERS and update CODEOWNERS --- .github/CODEOWNERS | 6 ++++-- MAINTAINERS | 3 +++ 2 files changed, 7 insertions(+), 2 deletions(-) create mode 100644 MAINTAINERS diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 47d0e56..da3f739 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1,6 +1,8 @@ -# Default owner should be a Pusher cloud-team member unless overridden by later -# rules in this file +# Default owner should be a Pusher cloud-team member or another maintainer +# unless overridden by later rules in this file * @pusher/cloud-team +* @syscll +* @steakunderscore # login.gov provider # Note: If @timothy-spencer terms out of his appointment, your best bet diff --git a/MAINTAINERS b/MAINTAINERS new file mode 100644 index 0000000..25fb475 --- /dev/null +++ b/MAINTAINERS @@ -0,0 +1,3 @@ +Joel Speed (@JoelSpeed) +Dan Bond (@syscll) +Henry Jenkins (@steakunderscore) From 23309adc7cc6a3e7bc587605d935e974315305f5 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Wed, 24 Jul 2019 09:21:08 +0100 Subject: [PATCH 08/19] Fix CODEOWNERS file --- .github/CODEOWNERS | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index da3f739..a6f7701 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1,8 +1,6 @@ # Default owner should be a Pusher cloud-team member or another maintainer # unless overridden by later rules in this file -* @pusher/cloud-team -* @syscll -* @steakunderscore +* @pusher/cloud-team @syscll @steakunderscore # login.gov provider # Note: If @timothy-spencer terms out of his appointment, your best bet From 1ab63304a1231705fa82669b020f599ef746f6b4 Mon Sep 17 00:00:00 2001 From: Reilly Brogan Date: Sat, 3 Aug 2019 13:22:42 -0500 Subject: [PATCH 09/19] Fix a bunch of places where the repo link was incorrect --- CHANGELOG.md | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74f09c2..9610a94 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,18 +31,18 @@ ## Changes since v3.2.0 -- [#178](https://github.com/pusher/outh2_proxy/pull/178) Add Silence Ping Logging and Exclude Logging Paths flags (@kskewes) -- [#209](https://github.com/pusher/outh2_proxy/pull/209) Improve docker build caching of layers (@dekimsey) +- [#178](https://github.com/pusher/oauth2_proxy/pull/178) Add Silence Ping Logging and Exclude Logging Paths flags (@kskewes) +- [#209](https://github.com/pusher/oauth2_proxy/pull/209) Improve docker build caching of layers (@dekimsey) - [#186](https://github.com/pusher/oauth2_proxy/pull/186) Make config consistent (@JoelSpeed) - [#187](https://github.com/pusher/oauth2_proxy/pull/187) Move root packages to pkg folder (@JoelSpeed) - [#65](https://github.com/pusher/oauth2_proxy/pull/65) Improvements to authenticate requests with a JWT bearer token in the `Authorization` header via the `-skip-jwt-bearer-token` options. - Additional verifiers can be configured via the `-extra-jwt-issuers` flag if the JWT issuers is either an OpenID provider or has a JWKS URL (e.g. `https://example.com/.well-known/jwks.json`). -- [#180](https://github.com/pusher/outh2_proxy/pull/180) Minor refactor of core proxying path (@aeijdenberg). -- [#175](https://github.com/pusher/outh2_proxy/pull/175) Bump go-oidc to v2.0.0 (@aeijdenberg). +- [#180](https://github.com/pusher/oauth2_proxy/pull/180) Minor refactor of core proxying path (@aeijdenberg). +- [#175](https://github.com/pusher/oauth2_proxy/pull/175) Bump go-oidc to v2.0.0 (@aeijdenberg). - Includes fix for potential signature checking issue when OIDC discovery is skipped. -- [#155](https://github.com/pusher/outh2_proxy/pull/155) Add RedisSessionStore implementation (@brianv0, @JoelSpeed) +- [#155](https://github.com/pusher/oauth2_proxy/pull/155) Add RedisSessionStore implementation (@brianv0, @JoelSpeed) - Implement flags to configure the redis session store - `-session-store-type=redis` Sets the store type to redis - `-redis-connection-url` Sets the Redis connection URL @@ -52,10 +52,10 @@ - Introduces the concept of a session ticket. Tickets are composed of the cookie name, a session ID, and a secret. - Redis Sessions are stored encrypted with a per-session secret - Added tests for server based session stores -- [#168](https://github.com/pusher/outh2_proxy/pull/168) Drop Go 1.11 support in Travis (@JoelSpeed) -- [#169](https://github.com/pusher/outh2_proxy/pull/169) Update Alpine to 3.9 (@kskewes) -- [#148](https://github.com/pusher/outh2_proxy/pull/148) Implement SessionStore interface within proxy (@JoelSpeed) -- [#147](https://github.com/pusher/outh2_proxy/pull/147) Add SessionStore interfaces and initial implementation (@JoelSpeed) +- [#168](https://github.com/pusher/oauth2_proxy/pull/168) Drop Go 1.11 support in Travis (@JoelSpeed) +- [#169](https://github.com/pusher/oauth2_proxy/pull/169) Update Alpine to 3.9 (@kskewes) +- [#148](https://github.com/pusher/oauth2_proxy/pull/148) Implement SessionStore interface within proxy (@JoelSpeed) +- [#147](https://github.com/pusher/oauth2_proxy/pull/147) Add SessionStore interfaces and initial implementation (@JoelSpeed) - Allows for multiple different session storage implementations including client and server side - Adds tests suite for interface to ensure consistency across implementations - Refactor some configuration options (around cookies) into packages @@ -83,8 +83,8 @@ - [#185](https://github.com/pusher/oauth2_proxy/pull/185) Fix an unsupported protocol scheme error during token validation when using the Azure provider (@jonas) - [#141](https://github.com/pusher/oauth2_proxy/pull/141) Check google group membership based on email address (@bchess) - Google Group membership is additionally checked via email address, allowing users outside a GSuite domain to be authorized. -- [#195](https://github.com/pusher/outh2_proxy/pull/195) Add `-banner` flag for overriding the banner line that is displayed (@steakunderscore) -- [#198](https://github.com/pusher/outh2_proxy/pull/198) Switch from gometalinter to golangci-lint (@steakunderscore) +- [#195](https://github.com/pusher/oauth2_proxy/pull/195) Add `-banner` flag for overriding the banner line that is displayed (@steakunderscore) +- [#198](https://github.com/pusher/oauth2_proxy/pull/198) Switch from gometalinter to golangci-lint (@steakunderscore) - [#159](https://github.com/pusher/oauth2_proxy/pull/159) Add option to skip the OIDC provider verified email check: `--insecure-oidc-allow-unverified-email` - [#210](https://github.com/pusher/oauth2_proxy/pull/210) Update base image from Alpine 3.9 to 3.10 (@steakunderscore) - [#211](https://github.com/pusher/oauth2_proxy/pull/211) Switch from dep to go modules (@steakunderscore) From d3462192934b1e19be561bccc793ebc4a3baedb2 Mon Sep 17 00:00:00 2001 From: Henry Jenkins Date: Sun, 4 Aug 2019 21:24:21 +0100 Subject: [PATCH 10/19] Remove dep from Travis CI Was missed from previous switch to go modules --- .travis.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 445eb11..aedf434 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,9 +3,6 @@ go: - 1.12.x install: # Fetch dependencies - - wget -O dep https://github.com/golang/dep/releases/download/v0.5.0/dep-linux-amd64 - - chmod +x dep - - mv dep $GOPATH/bin/dep - curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s -- -b $GOPATH/bin v1.17.1 script: - ./configure && make test From 8a24dd797fb1addaf31d87b9ab9648b5590acf3f Mon Sep 17 00:00:00 2001 From: hjenkins Date: Mon, 5 Aug 2019 09:25:14 +0100 Subject: [PATCH 11/19] Download modules in travis install step --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index aedf434..0537736 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,6 +4,7 @@ go: install: # Fetch dependencies - curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s -- -b $GOPATH/bin v1.17.1 + - GO111MODULE=on go mod download script: - ./configure && make test sudo: false From 7d910c0ae8eef5620fd6524a38521e953d1adb8e Mon Sep 17 00:00:00 2001 From: Justin Palpant Date: Tue, 6 Aug 2019 02:38:24 -0700 Subject: [PATCH 12/19] Check Google group membership with hasMember and get. (#224) * Check Google group membership with hasMember and get. This PR is an enhancement built on https://github.com/pusher/oauth2_proxy/pull/160. That PR reduces the number of calls to the Google Admin API and simplifies the code by using the hasMember method. It also supports checking membership in nested groups. However, the above message doesn't handle members who are not a part of the domain. The hasMember API returns a 400 for that case. As a fallback, when the API returns a 400, this change will try using the `get` API which works as expected for members who aren't a part of the domain. Supporting members who belong to the Google group but aren't part of the domain is a requested feature from https://github.com/pusher/oauth2_proxy/issues/95. https://developers.google.com/admin-sdk/directory/v1/reference/members/get Note that nested members who are not a part of the domain will not be correctly detected with this change. * Update CHANGELOG. * Fix incorrect JSON and stop escaping strings. * Add comments for each scenario. --- CHANGELOG.md | 5 ++- providers/google.go | 83 +++++++++++++--------------------------- providers/google_test.go | 51 ++++++++++++++++-------- 3 files changed, 66 insertions(+), 73 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9610a94..f979719 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,8 +31,9 @@ ## Changes since v3.2.0 -- [#178](https://github.com/pusher/oauth2_proxy/pull/178) Add Silence Ping Logging and Exclude Logging Paths flags (@kskewes) -- [#209](https://github.com/pusher/oauth2_proxy/pull/209) Improve docker build caching of layers (@dekimsey) +- [#224](https://github.com/pusher/oauth2_proxy/pull/224) Check Google group membership using hasMember to support nested groups and external users (@jpalpant) +- [#178](https://github.com/pusher/outh2_proxy/pull/178) Add Silence Ping Logging and Exclude Logging Paths flags (@kskewes) +- [#209](https://github.com/pusher/outh2_proxy/pull/209) Improve docker build caching of layers (@dekimsey) - [#186](https://github.com/pusher/oauth2_proxy/pull/186) Make config consistent (@JoelSpeed) - [#187](https://github.com/pusher/oauth2_proxy/pull/187) Move root packages to pkg folder (@JoelSpeed) - [#65](https://github.com/pusher/oauth2_proxy/pull/65) Improvements to authenticate requests with a JWT bearer token in the `Authorization` header via diff --git a/providers/google.go b/providers/google.go index 28c488c..4748631 100644 --- a/providers/google.go +++ b/providers/google.go @@ -189,73 +189,44 @@ func getAdminService(adminEmail string, credentialsReader io.Reader) *admin.Serv } func userInGroup(service *admin.Service, groups []string, email string) bool { - user, err := fetchUser(service, email) - if err != nil { - logger.Printf("Warning: unable to fetch user: %v", err) - user = nil - } - for _, group := range groups { - members, err := fetchGroupMembers(service, group) + // Use the HasMember API to checking for the user's presence in each group or nested subgroups + req := service.Members.HasMember(group, email) + r, err := req.Do() if err != nil { - if err, ok := err.(*googleapi.Error); ok && err.Code == 404 { - logger.Printf("error fetching members for group %s: group does not exist", group) - } else { - logger.Printf("error fetching group members: %v", err) - return false - } - } + err, ok := err.(*googleapi.Error) + if ok && err.Code == 404 { + logger.Printf("error checking membership in group %s: group does not exist", group) + } else if ok && err.Code == 400 { + // It is possible for Members.HasMember to return false even if the email is a group member. + // One case that can cause this is if the user email is from a different domain than the group, + // e.g. "member@otherdomain.com" in the group "group@mydomain.com" will result in a 400 error + // from the HasMember API. In that case, attempt to query the member object directly from the group. + req := service.Members.Get(group, email) + r, err := req.Do() - for _, member := range members { - if member.Email == email { - return true - } - if user == nil { - continue - } - switch member.Type { - case "CUSTOMER": - if member.Id == user.CustomerId { - return true - } - case "USER": - if member.Id == user.Id { + if err != nil { + logger.Printf("error using get API to check member %s of google group %s: user not in the group", email, group) + continue + } + + // If the non-domain user is found within the group, still verify that they are "ACTIVE". + // Do not count the user as belonging to a group if they have another status ("ARCHIVED", "SUSPENDED", or "UNKNOWN"). + if r.Status == "ACTIVE" { return true } + } else { + logger.Printf("error checking group membership: %v", err) } + continue + } + if r.IsMember { + return true } } return false } -func fetchUser(service *admin.Service, email string) (*admin.User, error) { - user, err := service.Users.Get(email).Do() - return user, err -} - -func fetchGroupMembers(service *admin.Service, group string) ([]*admin.Member, error) { - members := []*admin.Member{} - pageToken := "" - for { - req := service.Members.List(group) - if pageToken != "" { - req.PageToken(pageToken) - } - r, err := req.Do() - if err != nil { - return nil, err - } - for _, member := range r.Members { - members = append(members, member) - } - if r.NextPageToken == "" { - break - } - pageToken = r.NextPageToken - } - return members, nil -} - // ValidateGroup validates that the provided email exists in the configured Google // group(s). func (p *GoogleProvider) ValidateGroup(email string) bool { diff --git a/providers/google_test.go b/providers/google_test.go index 0c1725b..37b8326 100644 --- a/providers/google_test.go +++ b/providers/google_test.go @@ -1,6 +1,7 @@ package providers import ( + "context" "encoding/base64" "encoding/json" "fmt" @@ -12,6 +13,7 @@ import ( "github.com/stretchr/testify/assert" admin "google.golang.org/api/admin/directory/v1" + option "google.golang.org/api/option" ) func newRedeemServer(body []byte) (*url.URL, *httptest.Server) { @@ -185,34 +187,53 @@ func TestGoogleProviderGetEmailAddressEmailMissing(t *testing.T) { func TestGoogleProviderUserInGroup(t *testing.T) { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path == "/users/member-by-email@example.com" { - fmt.Fprintln(w, "{}") - } else if r.URL.Path == "/users/non-member-by-email@example.com" { - fmt.Fprintln(w, "{}") - } else if r.URL.Path == "/users/member-by-id@example.com" { - fmt.Fprintln(w, "{\"id\": \"member-id\"}") - } else if r.URL.Path == "/users/non-member-by-id@example.com" { - fmt.Fprintln(w, "{\"id\": \"non-member-id\"}") - } else if r.URL.Path == "/groups/group@example.com/members" { - fmt.Fprintln(w, "{\"members\": [{\"email\": \"member-by-email@example.com\"}, {\"id\": \"member-id\", \"type\": \"USER\"}]}") + if r.URL.Path == "/groups/group@example.com/hasMember/member-in-domain@example.com" { + fmt.Fprintln(w, `{"isMember": true}`) + } else if r.URL.Path == "/groups/group@example.com/hasMember/non-member-in-domain@example.com" { + fmt.Fprintln(w, `{"isMember": false}`) + } else if r.URL.Path == "/groups/group@example.com/hasMember/member-out-of-domain@otherexample.com" { + http.Error( + w, + `{"error": {"errors": [{"domain": "global","reason": "invalid","message": "Invalid Input: memberKey"}],"code": 400,"message": "Invalid Input: memberKey"}}`, + http.StatusBadRequest, + ) + } else if r.URL.Path == "/groups/group@example.com/hasMember/non-member-out-of-domain@otherexample.com" { + http.Error( + w, + `{"error": {"errors": [{"domain": "global","reason": "invalid","message": "Invalid Input: memberKey"}],"code": 400,"message": "Invalid Input: memberKey"}}`, + http.StatusBadRequest, + ) + } else if r.URL.Path == "/groups/group@example.com/members/non-member-out-of-domain@otherexample.com" { + // note that the client currently doesn't care what this response text or code is - any error here results in failure to match the group + http.Error( + w, + `{"error": {"errors": [{"domain": "global","reason": "notFound","message": "Resource Not Found: memberKey"}],"code": 404,"message": "Resource Not Found: memberKey"}}`, + http.StatusNotFound, + ) + } else if r.URL.Path == "/groups/group@example.com/members/member-out-of-domain@otherexample.com" { + fmt.Fprintln(w, + `{"kind": "admin#directory#member","etag":"12345","id":"1234567890","email": "member-out-of-domain@otherexample.com","role": "MEMBER","type": "USER","status": "ACTIVE","delivery_settings": "ALL_MAIL"}}`, + ) } })) defer ts.Close() client := ts.Client() - service, err := admin.New(client) + ctx := context.Background() + + service, err := admin.NewService(ctx, option.WithHTTPClient(client)) service.BasePath = ts.URL assert.Equal(t, nil, err) - result := userInGroup(service, []string{"group@example.com"}, "member-by-email@example.com") + result := userInGroup(service, []string{"group@example.com"}, "member-in-domain@example.com") assert.True(t, result) - result = userInGroup(service, []string{"group@example.com"}, "member-by-id@example.com") + result = userInGroup(service, []string{"group@example.com"}, "member-out-of-domain@otherexample.com") assert.True(t, result) - result = userInGroup(service, []string{"group@example.com"}, "non-member-by-id@example.com") + result = userInGroup(service, []string{"group@example.com"}, "non-member-in-domain@example.com") assert.False(t, result) - result = userInGroup(service, []string{"group@example.com"}, "non-member-by-email@example.com") + result = userInGroup(service, []string{"group@example.com"}, "non-member-out-of-domain@otherexample.com") assert.False(t, result) } From 5f9a65f6b1fc771fc2252877e5cb2f6a736ab2b4 Mon Sep 17 00:00:00 2001 From: hjenkins Date: Tue, 6 Aug 2019 12:16:03 +0100 Subject: [PATCH 13/19] Adds reference to slack channel in readme --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index b3a2806..9657e8f 100644 --- a/README.md +++ b/README.md @@ -38,6 +38,10 @@ Read the docs on our [Docs site](https://pusher.github.io/oauth2_proxy). ![OAuth2 Proxy Architecture](https://cloud.githubusercontent.com/assets/45028/8027702/bd040b7a-0d6a-11e5-85b9-f8d953d04f39.png) +## Getting Involved + +If you would like to reach out to the maintainers, come talk to us in the `#oauth2_proxy` channel in the [Gophers slack](http://gophers.slack.com/). + ## Contributing Please see our [Contributing](CONTRIBUTING.md) guidelines. From 4de49983fb682e15460d9cd1670e37a15f7782ab Mon Sep 17 00:00:00 2001 From: Alexander Overvoorde Date: Tue, 6 Aug 2019 13:20:54 +0200 Subject: [PATCH 14/19] Rework GitLab provider (#231) * Initial version of OIDC based GitLab provider * Add support for email domain check to GitLab provider * Add gitlab.com as default issuer for GitLab provider * Update documentation for GitLab provider * Update unit tests for new GitLab provider implementation * Update CHANGELOG for GitLab provider * Rename GitLab test access token as response to linter --- CHANGELOG.md | 6 + docs/2_auth.md | 10 +- docs/configuration/configuration.md | 1 + main.go | 1 + options.go | 24 +++ providers/gitlab.go | 266 ++++++++++++++++++++++++---- providers/gitlab_test.go | 172 +++++++++++------- 7 files changed, 374 insertions(+), 106 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f979719..8606b6b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ ## Breaking Changes +- [#231](https://github.com/pusher/oauth2_proxy/pull/231) Rework GitLab provider (@Overv) + - This PR changes the configuration options for the GitLab provider to use + a self-hosted instance. You now need to specify a `-oidc-issuer-url` rather than + explicit `-login-url`, `-redeem-url` and `-validate-url` parameters. + - [#186](https://github.com/pusher/oauth2_proxy/pull/186) Make config consistent - This PR changes configuration options so that all flags have a config counterpart of the same name but with underscores (`_`) in place of hyphens (`-`). @@ -32,6 +37,7 @@ ## Changes since v3.2.0 - [#224](https://github.com/pusher/oauth2_proxy/pull/224) Check Google group membership using hasMember to support nested groups and external users (@jpalpant) +- [#231](https://github.com/pusher/oauth2_proxy/pull/231) Add optional group membership and email domain checks to the GitLab provider (@Overv) - [#178](https://github.com/pusher/outh2_proxy/pull/178) Add Silence Ping Logging and Exclude Logging Paths flags (@kskewes) - [#209](https://github.com/pusher/outh2_proxy/pull/209) Improve docker build caching of layers (@dekimsey) - [#186](https://github.com/pusher/oauth2_proxy/pull/186) Make config consistent (@JoelSpeed) diff --git a/docs/2_auth.md b/docs/2_auth.md index 7a9bebd..e1a5ecd 100644 --- a/docs/2_auth.md +++ b/docs/2_auth.md @@ -103,13 +103,15 @@ If you are using GitHub enterprise, make sure you set the following to the appro ### GitLab Auth Provider -Whether you are using GitLab.com or self-hosting GitLab, follow [these steps to add an application](http://doc.gitlab.com/ce/integration/oauth_provider.html) +Whether you are using GitLab.com or self-hosting GitLab, follow [these steps to add an application](http://doc.gitlab.com/ce/integration/oauth_provider.html). Make sure to enable at least the `openid`, `profile` and `email` scopes. + +Restricting by group membership is possible with the following option: + + -gitlab-group="": restrict logins to members of any of these groups (slug), separated by a comma If you are using self-hosted GitLab, make sure you set the following to the appropriate URL: - -login-url="/oauth/authorize" - -redeem-url="/oauth/token" - -validate-url="/api/v4/user" + -oidc-issuer-url="" ### LinkedIn Auth Provider diff --git a/docs/configuration/configuration.md b/docs/configuration/configuration.md index f02d2ec..c9a70b1 100644 --- a/docs/configuration/configuration.md +++ b/docs/configuration/configuration.md @@ -49,6 +49,7 @@ Usage of oauth2_proxy: -gcp-healthchecks: will enable /liveness_check, /readiness_check, and / (with the proper user-agent) endpoints that will make it work well with GCP App Engine and GKE Ingresses (default false) -github-org string: restrict logins to members of this organisation -github-team string: restrict logins to members of any of these teams (slug), separated by a comma + -gitlab-group string: restrict logins to members of any of these groups (slug), separated by a comma -google-admin-email string: the google admin to impersonate for api calls -google-group value: restrict logins to members of this google group (may be given multiple times). -google-service-account-json string: the path to the service account json credentials diff --git a/main.go b/main.go index 97e2357..6db6025 100644 --- a/main.go +++ b/main.go @@ -57,6 +57,7 @@ func main() { flagSet.String("azure-tenant", "common", "go to a tenant-specific or common (tenant-independent) endpoint.") flagSet.String("github-org", "", "restrict logins to members of this organisation") flagSet.String("github-team", "", "restrict logins to members of this team") + flagSet.String("gitlab-group", "", "restrict logins to members of this group") flagSet.Var(&googleGroups, "google-group", "restrict logins to members of this google group (may be given multiple times).") flagSet.String("google-admin-email", "", "the google admin to impersonate for api calls") flagSet.String("google-service-account-json", "", "the path to the service account json credentials") diff --git a/options.go b/options.go index 7dcb12b..6969faa 100644 --- a/options.go +++ b/options.go @@ -46,6 +46,7 @@ type Options struct { WhitelistDomains []string `flag:"whitelist-domain" cfg:"whitelist_domains" env:"OAUTH2_PROXY_WHITELIST_DOMAINS"` GitHubOrg string `flag:"github-org" cfg:"github_org" env:"OAUTH2_PROXY_GITHUB_ORG"` GitHubTeam string `flag:"github-team" cfg:"github_team" env:"OAUTH2_PROXY_GITHUB_TEAM"` + GitLabGroup string `flag:"gitlab-group" cfg:"gitlab_group" env:"OAUTH2_PROXY_GITLAB_GROUP"` GoogleGroups []string `flag:"google-group" cfg:"google_group" env:"OAUTH2_PROXY_GOOGLE_GROUPS"` GoogleAdminEmail string `flag:"google-admin-email" cfg:"google_admin_email" env:"OAUTH2_PROXY_GOOGLE_ADMIN_EMAIL"` GoogleServiceAccountJSON string `flag:"google-service-account-json" cfg:"google_service_account_json" env:"OAUTH2_PROXY_GOOGLE_SERVICE_ACCOUNT_JSON"` @@ -410,6 +411,29 @@ func parseProviderInfo(o *Options, msgs []string) []string { } else { p.Verifier = o.oidcVerifier } + case *providers.GitLabProvider: + p.AllowUnverifiedEmail = o.InsecureOIDCAllowUnverifiedEmail + p.Group = o.GitLabGroup + p.EmailDomains = o.EmailDomains + + if o.oidcVerifier != nil { + p.Verifier = o.oidcVerifier + } else { + // Initialize with default verifier for gitlab.com + ctx := context.Background() + + provider, err := oidc.NewProvider(ctx, "https://gitlab.com") + if err != nil { + msgs = append(msgs, "failed to initialize oidc provider for gitlab.com") + } else { + p.Verifier = provider.Verifier(&oidc.Config{ + ClientID: o.ClientID, + }) + + p.LoginURL, msgs = parseURL(provider.Endpoint().AuthURL, "login", msgs) + p.RedeemURL, msgs = parseURL(provider.Endpoint().TokenURL, "redeem", msgs) + } + } case *providers.LoginGovProvider: p.AcrValues = o.AcrValues p.PubJWKURL, msgs = parseURL(o.PubJWKURL, "pubjwk", msgs) diff --git a/providers/gitlab.go b/providers/gitlab.go index 663ebd4..c32ebe8 100644 --- a/providers/gitlab.go +++ b/providers/gitlab.go @@ -1,62 +1,258 @@ package providers import ( + "context" + "encoding/json" + "fmt" + "io/ioutil" "net/http" - "net/url" + "strings" + "time" + oidc "github.com/coreos/go-oidc" "github.com/pusher/oauth2_proxy/pkg/apis/sessions" - "github.com/pusher/oauth2_proxy/pkg/logger" - "github.com/pusher/oauth2_proxy/pkg/requests" + "golang.org/x/oauth2" ) -// GitLabProvider represents an GitLab based Identity Provider +// GitLabProvider represents a GitLab based Identity Provider type GitLabProvider struct { *ProviderData + + Group string + EmailDomains []string + + Verifier *oidc.IDTokenVerifier + AllowUnverifiedEmail bool } // NewGitLabProvider initiates a new GitLabProvider func NewGitLabProvider(p *ProviderData) *GitLabProvider { p.ProviderName = "GitLab" - if p.LoginURL == nil || p.LoginURL.String() == "" { - p.LoginURL = &url.URL{ - Scheme: "https", - Host: "gitlab.com", - Path: "/oauth/authorize", - } - } - if p.RedeemURL == nil || p.RedeemURL.String() == "" { - p.RedeemURL = &url.URL{ - Scheme: "https", - Host: "gitlab.com", - Path: "/oauth/token", - } - } - if p.ValidateURL == nil || p.ValidateURL.String() == "" { - p.ValidateURL = &url.URL{ - Scheme: "https", - Host: "gitlab.com", - Path: "/api/v4/user", - } - } + if p.Scope == "" { - p.Scope = "read_user" + p.Scope = "openid email" } + return &GitLabProvider{ProviderData: p} } +// Redeem exchanges the OAuth2 authentication token for an ID token +func (p *GitLabProvider) Redeem(redirectURL, code string) (s *sessions.SessionState, err error) { + ctx := context.Background() + c := oauth2.Config{ + ClientID: p.ClientID, + ClientSecret: p.ClientSecret, + Endpoint: oauth2.Endpoint{ + TokenURL: p.RedeemURL.String(), + }, + RedirectURL: redirectURL, + } + token, err := c.Exchange(ctx, code) + if err != nil { + return nil, fmt.Errorf("token exchange: %v", err) + } + s, err = p.createSessionState(ctx, token) + if err != nil { + return nil, fmt.Errorf("unable to update session: %v", err) + } + return +} + +// RefreshSessionIfNeeded checks if the session has expired and uses the +// RefreshToken to fetch a new ID token if required +func (p *GitLabProvider) RefreshSessionIfNeeded(s *sessions.SessionState) (bool, error) { + if s == nil || s.ExpiresOn.After(time.Now()) || s.RefreshToken == "" { + return false, nil + } + + origExpiration := s.ExpiresOn + + err := p.redeemRefreshToken(s) + if err != nil { + return false, fmt.Errorf("unable to redeem refresh token: %v", err) + } + + fmt.Printf("refreshed id token %s (expired on %s)\n", s, origExpiration) + return true, nil +} + +func (p *GitLabProvider) redeemRefreshToken(s *sessions.SessionState) (err error) { + c := oauth2.Config{ + ClientID: p.ClientID, + ClientSecret: p.ClientSecret, + Endpoint: oauth2.Endpoint{ + TokenURL: p.RedeemURL.String(), + }, + } + ctx := context.Background() + t := &oauth2.Token{ + RefreshToken: s.RefreshToken, + Expiry: time.Now().Add(-time.Hour), + } + token, err := c.TokenSource(ctx, t).Token() + if err != nil { + return fmt.Errorf("failed to get token: %v", err) + } + newSession, err := p.createSessionState(ctx, token) + if err != nil { + return fmt.Errorf("unable to update session: %v", err) + } + s.AccessToken = newSession.AccessToken + s.IDToken = newSession.IDToken + s.RefreshToken = newSession.RefreshToken + s.CreatedAt = newSession.CreatedAt + s.ExpiresOn = newSession.ExpiresOn + s.Email = newSession.Email + return +} + +type gitlabUserInfo struct { + Username string `json:"nickname"` + Email string `json:"email"` + EmailVerified bool `json:"email_verified"` + Groups []string `json:"groups"` +} + +func (p *GitLabProvider) getUserInfo(s *sessions.SessionState) (*gitlabUserInfo, error) { + // Retrieve user info JSON + // https://docs.gitlab.com/ee/integration/openid_connect_provider.html#shared-information + + // Build user info url from login url of GitLab instance + userInfoURL := *p.LoginURL + userInfoURL.Path = "/oauth/userinfo" + + req, err := http.NewRequest("GET", userInfoURL.String(), nil) + if err != nil { + return nil, fmt.Errorf("failed to create user info request: %v", err) + } + req.Header.Set("Authorization", "Bearer "+s.AccessToken) + + resp, err := http.DefaultClient.Do(req) + if err != nil { + return nil, fmt.Errorf("failed to perform user info request: %v", err) + } + var body []byte + body, err = ioutil.ReadAll(resp.Body) + resp.Body.Close() + if err != nil { + return nil, fmt.Errorf("failed to read user info response: %v", err) + } + + if resp.StatusCode != 200 { + return nil, fmt.Errorf("got %d during user info request: %s", resp.StatusCode, body) + } + + var userInfo gitlabUserInfo + err = json.Unmarshal(body, &userInfo) + if err != nil { + return nil, fmt.Errorf("failed to parse user info: %v", err) + } + + return &userInfo, nil +} + +func (p *GitLabProvider) verifyGroupMembership(userInfo *gitlabUserInfo) error { + if p.Group == "" { + return nil + } + + // Collect user group memberships + membershipSet := make(map[string]bool) + for _, group := range userInfo.Groups { + membershipSet[group] = true + } + + // Find a valid group that they are a member of + validGroups := strings.Split(p.Group, " ") + for _, validGroup := range validGroups { + if _, ok := membershipSet[validGroup]; ok { + return nil + } + } + + return fmt.Errorf("user is not a member of '%s'", p.Group) +} + +func (p *GitLabProvider) verifyEmailDomain(userInfo *gitlabUserInfo) error { + if len(p.EmailDomains) == 0 || p.EmailDomains[0] == "*" { + return nil + } + + for _, domain := range p.EmailDomains { + if strings.HasSuffix(userInfo.Email, domain) { + return nil + } + } + + return fmt.Errorf("user email is not one of the valid domains '%v'", p.EmailDomains) +} + +func (p *GitLabProvider) createSessionState(ctx context.Context, token *oauth2.Token) (*sessions.SessionState, error) { + rawIDToken, ok := token.Extra("id_token").(string) + if !ok { + return nil, fmt.Errorf("token response did not contain an id_token") + } + + // Parse and verify ID Token payload. + idToken, err := p.Verifier.Verify(ctx, rawIDToken) + if err != nil { + return nil, fmt.Errorf("could not verify id_token: %v", err) + } + + return &sessions.SessionState{ + AccessToken: token.AccessToken, + IDToken: rawIDToken, + RefreshToken: token.RefreshToken, + CreatedAt: time.Now(), + ExpiresOn: idToken.Expiry, + }, nil +} + +// ValidateSessionState checks that the session's IDToken is still valid +func (p *GitLabProvider) ValidateSessionState(s *sessions.SessionState) bool { + ctx := context.Background() + _, err := p.Verifier.Verify(ctx, s.IDToken) + if err != nil { + return false + } + + return true +} + // GetEmailAddress returns the Account email address func (p *GitLabProvider) GetEmailAddress(s *sessions.SessionState) (string, error) { + // Retrieve user info + userInfo, err := p.getUserInfo(s) + if err != nil { + return "", fmt.Errorf("failed to retrieve user info: %v", err) + } - req, err := http.NewRequest("GET", - p.ValidateURL.String()+"?access_token="+s.AccessToken, nil) - if err != nil { - logger.Printf("failed building request %s", err) - return "", err + // Check if email is verified + if !p.AllowUnverifiedEmail && !userInfo.EmailVerified { + return "", fmt.Errorf("user email is not verified") } - json, err := requests.Request(req) + + // Check if email has valid domain + err = p.verifyEmailDomain(userInfo) if err != nil { - logger.Printf("failed making request %s", err) - return "", err + return "", fmt.Errorf("email domain check failed: %v", err) } - return json.Get("email").String() + + // Check group membership + err = p.verifyGroupMembership(userInfo) + if err != nil { + return "", fmt.Errorf("group membership check failed: %v", err) + } + + return userInfo.Email, nil +} + +// GetUserName returns the Account user name +func (p *GitLabProvider) GetUserName(s *sessions.SessionState) (string, error) { + userInfo, err := p.getUserInfo(s) + if err != nil { + return "", fmt.Errorf("failed to retrieve user info: %v", err) + } + + return userInfo.Username, nil } diff --git a/providers/gitlab_test.go b/providers/gitlab_test.go index 112eb89..f75c4bf 100644 --- a/providers/gitlab_test.go +++ b/providers/gitlab_test.go @@ -25,104 +25,142 @@ func testGitLabProvider(hostname string) *GitLabProvider { updateURL(p.Data().ProfileURL, hostname) updateURL(p.Data().ValidateURL, hostname) } + return p } -func testGitLabBackend(payload string) *httptest.Server { - path := "/api/v4/user" - query := "access_token=imaginary_access_token" +func testGitLabBackend() *httptest.Server { + userInfo := ` + { + "nickname": "FooBar", + "email": "foo@bar.com", + "email_verified": false, + "groups": ["foo", "bar"] + } + ` + authHeader := "Bearer gitlab_access_token" return httptest.NewServer(http.HandlerFunc( func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path != path || r.URL.RawQuery != query { - w.WriteHeader(404) + if r.URL.Path == "/oauth/userinfo" { + if r.Header["Authorization"][0] == authHeader { + w.WriteHeader(200) + w.Write([]byte(userInfo)) + } else { + w.WriteHeader(401) + } } else { - w.WriteHeader(200) - w.Write([]byte(payload)) + w.WriteHeader(404) } })) } -func TestGitLabProviderDefaults(t *testing.T) { - p := testGitLabProvider("") - assert.NotEqual(t, nil, p) - assert.Equal(t, "GitLab", p.Data().ProviderName) - assert.Equal(t, "https://gitlab.com/oauth/authorize", - p.Data().LoginURL.String()) - assert.Equal(t, "https://gitlab.com/oauth/token", - p.Data().RedeemURL.String()) - assert.Equal(t, "https://gitlab.com/api/v4/user", - p.Data().ValidateURL.String()) - assert.Equal(t, "read_user", p.Data().Scope) -} - -func TestGitLabProviderOverrides(t *testing.T) { - p := NewGitLabProvider( - &ProviderData{ - LoginURL: &url.URL{ - Scheme: "https", - Host: "example.com", - Path: "/oauth/auth"}, - RedeemURL: &url.URL{ - Scheme: "https", - Host: "example.com", - Path: "/oauth/token"}, - ValidateURL: &url.URL{ - Scheme: "https", - Host: "example.com", - Path: "/api/v4/user"}, - Scope: "profile"}) - assert.NotEqual(t, nil, p) - assert.Equal(t, "GitLab", p.Data().ProviderName) - assert.Equal(t, "https://example.com/oauth/auth", - p.Data().LoginURL.String()) - assert.Equal(t, "https://example.com/oauth/token", - p.Data().RedeemURL.String()) - assert.Equal(t, "https://example.com/api/v4/user", - p.Data().ValidateURL.String()) - assert.Equal(t, "profile", p.Data().Scope) -} - -func TestGitLabProviderGetEmailAddress(t *testing.T) { - b := testGitLabBackend("{\"email\": \"michael.bland@gsa.gov\"}") +func TestGitLabProviderBadToken(t *testing.T) { + b := testGitLabBackend() defer b.Close() bURL, _ := url.Parse(b.URL) p := testGitLabProvider(bURL.Host) - session := &sessions.SessionState{AccessToken: "imaginary_access_token"} + session := &sessions.SessionState{AccessToken: "unexpected_gitlab_access_token"} + _, err := p.GetEmailAddress(session) + assert.NotEqual(t, nil, err) +} + +func TestGitLabProviderUnverifiedEmailDenied(t *testing.T) { + b := testGitLabBackend() + defer b.Close() + + bURL, _ := url.Parse(b.URL) + p := testGitLabProvider(bURL.Host) + + session := &sessions.SessionState{AccessToken: "gitlab_access_token"} + _, err := p.GetEmailAddress(session) + assert.NotEqual(t, nil, err) +} + +func TestGitLabProviderUnverifiedEmailAllowed(t *testing.T) { + b := testGitLabBackend() + defer b.Close() + + bURL, _ := url.Parse(b.URL) + p := testGitLabProvider(bURL.Host) + p.AllowUnverifiedEmail = true + + session := &sessions.SessionState{AccessToken: "gitlab_access_token"} email, err := p.GetEmailAddress(session) assert.Equal(t, nil, err) - assert.Equal(t, "michael.bland@gsa.gov", email) + assert.Equal(t, "foo@bar.com", email) } -// Note that trying to trigger the "failed building request" case is not -// practical, since the only way it can fail is if the URL fails to parse. -func TestGitLabProviderGetEmailAddressFailedRequest(t *testing.T) { - b := testGitLabBackend("unused payload") +func TestGitLabProviderUsername(t *testing.T) { + b := testGitLabBackend() defer b.Close() bURL, _ := url.Parse(b.URL) p := testGitLabProvider(bURL.Host) + p.AllowUnverifiedEmail = true - // We'll trigger a request failure by using an unexpected access - // token. Alternatively, we could allow the parsing of the payload as - // JSON to fail. - session := &sessions.SessionState{AccessToken: "unexpected_access_token"} - email, err := p.GetEmailAddress(session) - assert.NotEqual(t, nil, err) - assert.Equal(t, "", email) + session := &sessions.SessionState{AccessToken: "gitlab_access_token"} + username, err := p.GetUserName(session) + assert.Equal(t, nil, err) + assert.Equal(t, "FooBar", username) } -func TestGitLabProviderGetEmailAddressEmailNotPresentInPayload(t *testing.T) { - b := testGitLabBackend("{\"foo\": \"bar\"}") +func TestGitLabProviderGroupMembershipValid(t *testing.T) { + b := testGitLabBackend() defer b.Close() bURL, _ := url.Parse(b.URL) p := testGitLabProvider(bURL.Host) + p.AllowUnverifiedEmail = true + p.Group = "foo" - session := &sessions.SessionState{AccessToken: "imaginary_access_token"} + session := &sessions.SessionState{AccessToken: "gitlab_access_token"} email, err := p.GetEmailAddress(session) - assert.NotEqual(t, nil, err) - assert.Equal(t, "", email) + assert.Equal(t, nil, err) + assert.Equal(t, "foo@bar.com", email) +} + +func TestGitLabProviderGroupMembershipMissing(t *testing.T) { + b := testGitLabBackend() + defer b.Close() + + bURL, _ := url.Parse(b.URL) + p := testGitLabProvider(bURL.Host) + p.AllowUnverifiedEmail = true + p.Group = "baz" + + session := &sessions.SessionState{AccessToken: "gitlab_access_token"} + _, err := p.GetEmailAddress(session) + assert.NotEqual(t, nil, err) +} + +func TestGitLabProviderEmailDomainValid(t *testing.T) { + b := testGitLabBackend() + defer b.Close() + + bURL, _ := url.Parse(b.URL) + p := testGitLabProvider(bURL.Host) + p.AllowUnverifiedEmail = true + p.EmailDomains = []string{"bar.com"} + + session := &sessions.SessionState{AccessToken: "gitlab_access_token"} + email, err := p.GetEmailAddress(session) + assert.Equal(t, nil, err) + assert.Equal(t, "foo@bar.com", email) +} + +func TestGitLabProviderEmailDomainInvalid(t *testing.T) { + b := testGitLabBackend() + defer b.Close() + + bURL, _ := url.Parse(b.URL) + p := testGitLabProvider(bURL.Host) + p.AllowUnverifiedEmail = true + p.EmailDomains = []string{"baz.com"} + + session := &sessions.SessionState{AccessToken: "gitlab_access_token"} + _, err := p.GetEmailAddress(session) + assert.NotEqual(t, nil, err) } From d85660248cb10abeb0c2840fcaf45dbf699b4e91 Mon Sep 17 00:00:00 2001 From: mikesiegel Date: Wed, 7 Aug 2019 06:57:18 -0400 Subject: [PATCH 15/19] Adding docs for how to configure Okta for the OIDC provider (#235) * Adding documentation for Okta OIDC provider. * additional clean up. * Clearer heading * Forgot a word. * updated documentation based on ReillyProcentive review. * Per steakunderscore review: removed defaults. Removed extra hardening steps (expiration, https only etc) not directly related to setting up Okta w/ OIDC --- docs/2_auth.md | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/docs/2_auth.md b/docs/2_auth.md index e1a5ecd..eba5f0c 100644 --- a/docs/2_auth.md +++ b/docs/2_auth.md @@ -146,6 +146,56 @@ OpenID Connect is a spec for OAUTH 2.0 + identity that is implemented by many ma -cookie-secure=false -email-domain example.com +The OpenID Connect Provider (OIDC) can also be used to connect to other Identity Providers such as Okta. To configure the OIDC provider for Okta, perform +the following steps: + +#### Configuring the OIDC Provider with Okta + +1. Log in to Okta using an administrative account. It is suggested you try this in preview first, `example.oktapreview.com` +2. (OPTIONAL) If you want to configure authorization scopes and claims to be passed on to multiple applications, +you may wish to configure an authorization server for each application. Otherwise, the provided `default` will work. +* Navigate to **Security** then select **API** +* Click **Add Authorization Server**, if this option is not available you may require an additional license for a custom authorization server. +* Fill out the **Name** with something to describe the application you are protecting. e.g. 'Example App'. +* For **Audience**, pick the URL of the application you wish to protect: https://example.corp.com +* Fill out a **Description** +* Add any **Access Policies** you wish to configure to limit application access. +* The default settings will work for other options. +[See Okta documentation for more information on Authorization Servers](https://developer.okta.com/docs/guides/customize-authz-server/overview/) +3. Navigate to **Applications** then select **Add Application**. +* Select **Web** for the **Platform** setting. +* Select **OpenID Connect** and click **Create** +* Pick an **Application Name** such as `Example App`. +* Set the **Login redirect URI** to `https://example.corp.com`. +* Under **General** set the **Allowed grant types** to `Authorization Code` and `Refresh Token`. +* Leave the rest as default, taking note of the `Client ID` and `Client Secret`. +* Under **Assignments** select the users or groups you wish to access your application. +4. Create a configuration file like the following: + +``` +provider = "oidc" +redirect_url = "https://example.corp.com" +oidc_issuer_url = "https://corp.okta.com/oauth2/abCd1234" +upstreams = [ + "https://example.corp.com" +] +email_domains = [ + "corp.com" +] +client_id = "XXXXX" +client_secret = "YYYYY" +pass_access_token = true +cookie_secret = "ZZZZZ" +skip_provider_button = true +``` + +The `oidc_issuer_url` is based on URL from your **Authorization Server**'s **Issuer** field in step 2, or simply https://corp.okta.com +The `client_id` and `client_secret` are configured in the application settings. +Generate a unique `client_secret` to encrypt the cookie. + +Then you can start the oauth2_proxy with `./oauth2_proxy -config /etc/example.cfg` + + ### login.gov Provider login.gov is an OIDC provider for the US Government. From 7134d22bcc9d7504e6082a6cbef97edd7390c71a Mon Sep 17 00:00:00 2001 From: jansinger Date: Wed, 7 Aug 2019 18:48:53 +0200 Subject: [PATCH 16/19] New flag "-ssl-upstream-insecure-skip-validation" (#234) * New flag "-ssl-upstream-insecure-skip-validation" to skip SSL validation for upstreams with self generated / invalid SSL certificates. * Fix tests for modified NewReverseProxy method. * Added change to the changelog. * Remove duplicate entries from changelog. --- CHANGELOG.md | 1 + docs/configuration/configuration.md | 3 ++- main.go | 3 ++- oauthproxy.go | 12 ++++++++--- oauthproxy_test.go | 4 ++-- options.go | 33 +++++++++++++++-------------- 6 files changed, 33 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8606b6b..81ee496 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ ## Changes since v3.2.0 +- [#234](https://github.com/pusher/oauth2_proxy/pull/234) Added option `-ssl-upstream-insecure-skip-validation` to skip validation of upstream SSL certificates (@jansinger) - [#224](https://github.com/pusher/oauth2_proxy/pull/224) Check Google group membership using hasMember to support nested groups and external users (@jpalpant) - [#231](https://github.com/pusher/oauth2_proxy/pull/231) Add optional group membership and email domain checks to the GitLab provider (@Overv) - [#178](https://github.com/pusher/outh2_proxy/pull/178) Add Silence Ping Logging and Exclude Logging Paths flags (@kskewes) diff --git a/docs/configuration/configuration.md b/docs/configuration/configuration.md index c9a70b1..5bef512 100644 --- a/docs/configuration/configuration.md +++ b/docs/configuration/configuration.md @@ -99,7 +99,8 @@ Usage of oauth2_proxy: -skip-jwt-bearer-tokens: will skip requests that have verified JWT bearer tokens -skip-oidc-discovery: bypass OIDC endpoint discovery. login-url, redeem-url and oidc-jwks-url must be configured in this case -skip-provider-button: will skip sign-in-page to directly reach the next step: oauth/start - -ssl-insecure-skip-verify: skip validation of certificates presented when using HTTPS + -ssl-insecure-skip-verify: skip validation of certificates presented when using HTTPS providers + -ssl-upstream-insecure-skip-verify: skip validation of certificates presented when using HTTPS upstreams -standard-logging: Log standard runtime information (default true) -standard-logging-format string: Template for standard log lines (see "Logging Configuration" paragraph below) -tls-cert-file string: path to certificate file diff --git a/main.go b/main.go index 6db6025..823a16d 100644 --- a/main.go +++ b/main.go @@ -47,7 +47,8 @@ func main() { flagSet.Var(&skipAuthRegex, "skip-auth-regex", "bypass authentication for requests path's that match (may be given multiple times)") flagSet.Bool("skip-provider-button", false, "will skip sign-in-page to directly reach the next step: oauth/start") flagSet.Bool("skip-auth-preflight", false, "will skip authentication for OPTIONS requests") - flagSet.Bool("ssl-insecure-skip-verify", false, "skip validation of certificates presented when using HTTPS") + flagSet.Bool("ssl-insecure-skip-verify", false, "skip validation of certificates presented when using HTTPS providers") + flagSet.Bool("ssl-upstream-insecure-skip-verify", false, "skip validation of certificates presented when using HTTPS upstreams") flagSet.Duration("flush-interval", time.Duration(1)*time.Second, "period between response flushing when streaming responses") flagSet.Bool("skip-jwt-bearer-tokens", false, "will skip requests that have verified JWT bearer tokens (default false)") flagSet.Var(&jwtIssuers, "extra-jwt-issuers", "if skip-jwt-bearer-tokens is set, a list of extra JWT issuer=audience pairs (where the issuer URL has a .well-known/openid-configuration or a .well-known/jwks.json)") diff --git a/oauthproxy.go b/oauthproxy.go index 5ef7a39..365a14e 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -2,6 +2,7 @@ package main import ( "context" + "crypto/tls" b64 "encoding/base64" "errors" "fmt" @@ -128,9 +129,14 @@ func (u *UpstreamProxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { // NewReverseProxy creates a new reverse proxy for proxying requests to upstream // servers -func NewReverseProxy(target *url.URL, flushInterval time.Duration) (proxy *httputil.ReverseProxy) { +func NewReverseProxy(target *url.URL, opts *Options) (proxy *httputil.ReverseProxy) { proxy = httputil.NewSingleHostReverseProxy(target) - proxy.FlushInterval = flushInterval + proxy.FlushInterval = opts.FlushInterval + if opts.SSLUpstreamInsecureSkipVerify { + proxy.Transport = &http.Transport{ + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, + } + } return proxy } @@ -163,7 +169,7 @@ func NewFileServer(path string, filesystemPath string) (proxy http.Handler) { // NewWebSocketOrRestReverseProxy creates a reverse proxy for REST or websocket based on url func NewWebSocketOrRestReverseProxy(u *url.URL, opts *Options, auth hmacauth.HmacAuth) http.Handler { u.Path = "" - proxy := NewReverseProxy(u, opts.FlushInterval) + proxy := NewReverseProxy(u, opts) if !opts.PassHostHeader { setProxyUpstreamHostHeader(proxy, u) } else { diff --git a/oauthproxy_test.go b/oauthproxy_test.go index c3d422c..8dd3adf 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -122,7 +122,7 @@ func TestNewReverseProxy(t *testing.T) { backendHost := net.JoinHostPort(backendHostname, backendPort) proxyURL, _ := url.Parse(backendURL.Scheme + "://" + backendHost + "/") - proxyHandler := NewReverseProxy(proxyURL, time.Second) + proxyHandler := NewReverseProxy(proxyURL, &Options{FlushInterval: time.Second}) setProxyUpstreamHostHeader(proxyHandler, proxyURL) frontend := httptest.NewServer(proxyHandler) defer frontend.Close() @@ -144,7 +144,7 @@ func TestEncodedSlashes(t *testing.T) { defer backend.Close() b, _ := url.Parse(backend.URL) - proxyHandler := NewReverseProxy(b, time.Second) + proxyHandler := NewReverseProxy(b, &Options{FlushInterval: time.Second}) setProxyDirector(proxyHandler) frontend := httptest.NewServer(proxyHandler) defer frontend.Close() diff --git a/options.go b/options.go index 6969faa..03f5ff3 100644 --- a/options.go +++ b/options.go @@ -62,22 +62,23 @@ type Options struct { // Embed SessionOptions options.SessionOptions - Upstreams []string `flag:"upstream" cfg:"upstreams" env:"OAUTH2_PROXY_UPSTREAMS"` - SkipAuthRegex []string `flag:"skip-auth-regex" cfg:"skip_auth_regex" env:"OAUTH2_PROXY_SKIP_AUTH_REGEX"` - SkipJwtBearerTokens bool `flag:"skip-jwt-bearer-tokens" cfg:"skip_jwt_bearer_tokens" env:"OAUTH2_PROXY_SKIP_JWT_BEARER_TOKENS"` - ExtraJwtIssuers []string `flag:"extra-jwt-issuers" cfg:"extra_jwt_issuers" env:"OAUTH2_PROXY_EXTRA_JWT_ISSUERS"` - PassBasicAuth bool `flag:"pass-basic-auth" cfg:"pass_basic_auth" env:"OAUTH2_PROXY_PASS_BASIC_AUTH"` - BasicAuthPassword string `flag:"basic-auth-password" cfg:"basic_auth_password" env:"OAUTH2_PROXY_BASIC_AUTH_PASSWORD"` - PassAccessToken bool `flag:"pass-access-token" cfg:"pass_access_token" env:"OAUTH2_PROXY_PASS_ACCESS_TOKEN"` - PassHostHeader bool `flag:"pass-host-header" cfg:"pass_host_header" env:"OAUTH2_PROXY_PASS_HOST_HEADER"` - SkipProviderButton bool `flag:"skip-provider-button" cfg:"skip_provider_button" env:"OAUTH2_PROXY_SKIP_PROVIDER_BUTTON"` - PassUserHeaders bool `flag:"pass-user-headers" cfg:"pass_user_headers" env:"OAUTH2_PROXY_PASS_USER_HEADERS"` - SSLInsecureSkipVerify bool `flag:"ssl-insecure-skip-verify" cfg:"ssl_insecure_skip_verify" env:"OAUTH2_PROXY_SSL_INSECURE_SKIP_VERIFY"` - SetXAuthRequest bool `flag:"set-xauthrequest" cfg:"set_xauthrequest" env:"OAUTH2_PROXY_SET_XAUTHREQUEST"` - SetAuthorization bool `flag:"set-authorization-header" cfg:"set_authorization_header" env:"OAUTH2_PROXY_SET_AUTHORIZATION_HEADER"` - PassAuthorization bool `flag:"pass-authorization-header" cfg:"pass_authorization_header" env:"OAUTH2_PROXY_PASS_AUTHORIZATION_HEADER"` - SkipAuthPreflight bool `flag:"skip-auth-preflight" cfg:"skip_auth_preflight" env:"OAUTH2_PROXY_SKIP_AUTH_PREFLIGHT"` - FlushInterval time.Duration `flag:"flush-interval" cfg:"flush_interval" env:"OAUTH2_PROXY_FLUSH_INTERVAL"` + Upstreams []string `flag:"upstream" cfg:"upstreams" env:"OAUTH2_PROXY_UPSTREAMS"` + SkipAuthRegex []string `flag:"skip-auth-regex" cfg:"skip_auth_regex" env:"OAUTH2_PROXY_SKIP_AUTH_REGEX"` + SkipJwtBearerTokens bool `flag:"skip-jwt-bearer-tokens" cfg:"skip_jwt_bearer_tokens" env:"OAUTH2_PROXY_SKIP_JWT_BEARER_TOKENS"` + ExtraJwtIssuers []string `flag:"extra-jwt-issuers" cfg:"extra_jwt_issuers" env:"OAUTH2_PROXY_EXTRA_JWT_ISSUERS"` + PassBasicAuth bool `flag:"pass-basic-auth" cfg:"pass_basic_auth" env:"OAUTH2_PROXY_PASS_BASIC_AUTH"` + BasicAuthPassword string `flag:"basic-auth-password" cfg:"basic_auth_password" env:"OAUTH2_PROXY_BASIC_AUTH_PASSWORD"` + PassAccessToken bool `flag:"pass-access-token" cfg:"pass_access_token" env:"OAUTH2_PROXY_PASS_ACCESS_TOKEN"` + PassHostHeader bool `flag:"pass-host-header" cfg:"pass_host_header" env:"OAUTH2_PROXY_PASS_HOST_HEADER"` + SkipProviderButton bool `flag:"skip-provider-button" cfg:"skip_provider_button" env:"OAUTH2_PROXY_SKIP_PROVIDER_BUTTON"` + PassUserHeaders bool `flag:"pass-user-headers" cfg:"pass_user_headers" env:"OAUTH2_PROXY_PASS_USER_HEADERS"` + SSLInsecureSkipVerify bool `flag:"ssl-insecure-skip-verify" cfg:"ssl_insecure_skip_verify" env:"OAUTH2_PROXY_SSL_INSECURE_SKIP_VERIFY"` + SSLUpstreamInsecureSkipVerify bool `flag:"ssl-upstream-insecure-skip-verify" cfg:"ssl_upstream_insecure_skip_verify" env:"OAUTH2_PROXY_SSL_UPSTREAM_INSECURE_SKIP_VERIFY"` + SetXAuthRequest bool `flag:"set-xauthrequest" cfg:"set_xauthrequest" env:"OAUTH2_PROXY_SET_XAUTHREQUEST"` + SetAuthorization bool `flag:"set-authorization-header" cfg:"set_authorization_header" env:"OAUTH2_PROXY_SET_AUTHORIZATION_HEADER"` + PassAuthorization bool `flag:"pass-authorization-header" cfg:"pass_authorization_header" env:"OAUTH2_PROXY_PASS_AUTHORIZATION_HEADER"` + SkipAuthPreflight bool `flag:"skip-auth-preflight" cfg:"skip_auth_preflight" env:"OAUTH2_PROXY_SKIP_AUTH_PREFLIGHT"` + FlushInterval time.Duration `flag:"flush-interval" cfg:"flush_interval" env:"OAUTH2_PROXY_FLUSH_INTERVAL"` // These options allow for other providers besides Google, with // potential overrides. From 02dfa87f118c99423d7ec5571272342b2c7d7d9f Mon Sep 17 00:00:00 2001 From: hjenkins Date: Wed, 7 Aug 2019 17:46:34 +0100 Subject: [PATCH 17/19] Fix typos in changelog --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 81ee496..3b4a00b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,8 +39,8 @@ - [#234](https://github.com/pusher/oauth2_proxy/pull/234) Added option `-ssl-upstream-insecure-skip-validation` to skip validation of upstream SSL certificates (@jansinger) - [#224](https://github.com/pusher/oauth2_proxy/pull/224) Check Google group membership using hasMember to support nested groups and external users (@jpalpant) - [#231](https://github.com/pusher/oauth2_proxy/pull/231) Add optional group membership and email domain checks to the GitLab provider (@Overv) -- [#178](https://github.com/pusher/outh2_proxy/pull/178) Add Silence Ping Logging and Exclude Logging Paths flags (@kskewes) -- [#209](https://github.com/pusher/outh2_proxy/pull/209) Improve docker build caching of layers (@dekimsey) +- [#178](https://github.com/pusher/oauth2_proxy/pull/178) Add Silence Ping Logging and Exclude Logging Paths flags (@kskewes) +- [#209](https://github.com/pusher/oauth2_proxy/pull/209) Improve docker build caching of layers (@dekimsey) - [#186](https://github.com/pusher/oauth2_proxy/pull/186) Make config consistent (@JoelSpeed) - [#187](https://github.com/pusher/oauth2_proxy/pull/187) Move root packages to pkg folder (@JoelSpeed) - [#65](https://github.com/pusher/oauth2_proxy/pull/65) Improvements to authenticate requests with a JWT bearer token in the `Authorization` header via From 9e37de53e39b47854839c73565fd44d15a86f084 Mon Sep 17 00:00:00 2001 From: Vitalii Tverdokhlib Date: Sun, 11 Aug 2019 14:55:19 +0300 Subject: [PATCH 18/19] docs: fix path to oauth2_proxy.cfg --- docs/configuration/configuration.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/configuration/configuration.md b/docs/configuration/configuration.md index 5bef512..ea677d5 100644 --- a/docs/configuration/configuration.md +++ b/docs/configuration/configuration.md @@ -14,7 +14,7 @@ To generate a strong cookie secret use `python -c 'import os,base64; print base6 ### Config File -An example [oauth2_proxy.cfg](contrib/oauth2_proxy.cfg.example) config file is in the contrib directory. It can be used by specifying `-config=/etc/oauth2_proxy.cfg` +An example [oauth2_proxy.cfg](https://github.com/pusher/oauth2_proxy/blob/master/contrib/oauth2_proxy.cfg.example) config file is in the contrib directory. It can be used by specifying `-config=/etc/oauth2_proxy.cfg` ### Command Line Options From fb52bdb90cbf638b65c900c05582eace43b23223 Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Tue, 13 Aug 2019 12:42:23 +0200 Subject: [PATCH 19/19] Fix some typos --- docs/configuration/configuration.md | 4 ++-- main.go | 4 ++-- oauthproxy.go | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/configuration/configuration.md b/docs/configuration/configuration.md index ea677d5..5e06883 100644 --- a/docs/configuration/configuration.md +++ b/docs/configuration/configuration.md @@ -82,8 +82,8 @@ Usage of oauth2_proxy: -redeem-url string: Token redemption endpoint -redirect-url string: the OAuth Redirect URL. ie: "https://internalapp.yourcompany.com/oauth2/callback" -redis-connection-url string: URL of redis server for redis session storage (eg: redis://HOST[:PORT]) - -redis-sentinel-master-name string: Redis sentinel master name. Used in conjuction with --redis-use-sentinel - -redis-sentinel-connection-urls: List of Redis sentinel conneciton URLs (eg redis://HOST[:PORT]). Used in conjuction with --redis-use-sentinel + -redis-sentinel-master-name string: Redis sentinel master name. Used in conjunction with --redis-use-sentinel + -redis-sentinel-connection-urls: List of Redis sentinel connection URLs (eg redis://HOST[:PORT]). Used in conjunction with --redis-use-sentinel -redis-use-sentinel: Connect to redis via sentinels. Must set --redis-sentinel-master-name and --redis-sentinel-connection-urls to use this feature (default: false) -request-logging: Log requests to stdout (default true) -request-logging-format: Template for request log lines (see "Logging Configuration" paragraph below) diff --git a/main.go b/main.go index 823a16d..872990a 100644 --- a/main.go +++ b/main.go @@ -86,8 +86,8 @@ func main() { flagSet.String("session-store-type", "cookie", "the session storage provider to use") flagSet.String("redis-connection-url", "", "URL of redis server for redis session storage (eg: redis://HOST[:PORT])") flagSet.Bool("redis-use-sentinel", false, "Connect to redis via sentinels. Must set --redis-sentinel-master-name and --redis-sentinel-connection-urls to use this feature") - flagSet.String("redis-sentinel-master-name", "", "Redis sentinel master name. Used in conjuction with --redis-use-sentinel") - flagSet.Var(&redisSentinelConnectionURLs, "redis-sentinel-connection-urls", "List of Redis sentinel connection URLs (eg redis://HOST[:PORT]). Used in conjuction with --redis-use-sentinel") + flagSet.String("redis-sentinel-master-name", "", "Redis sentinel master name. Used in conjunction with --redis-use-sentinel") + flagSet.Var(&redisSentinelConnectionURLs, "redis-sentinel-connection-urls", "List of Redis sentinel connection URLs (eg redis://HOST[:PORT]). Used in conjunction with --redis-use-sentinel") flagSet.String("logging-filename", "", "File to log requests to, empty for stdout") flagSet.Int("logging-max-size", 100, "Maximum size in megabytes of the log file before rotation") diff --git a/oauthproxy.go b/oauthproxy.go index 365a14e..f3d9fd9 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -898,7 +898,7 @@ func isAjax(req *http.Request) bool { return false } -// ErrorJSON returns the error code witht an application/json mime type +// ErrorJSON returns the error code with an application/json mime type func (p *OAuthProxy) ErrorJSON(rw http.ResponseWriter, code int) { rw.Header().Set("Content-Type", applicationJSON) rw.WriteHeader(code)