git.iko.soy/typeable/oauth2-proxy
on master help sign in
Authorize in Redeem callback flow
commitf21b3b8b200bf972d920ccc75b298c91c8279944 authorNick Meves <nick.meves@greenhouse.io> dateFri 23 Oct 2020, 20:53 (6 years ago) parent1b3b004
4 files changed+29−17
compare with master
unifiedsplit
CHANGELOG.md+1−1 oauthproxy.go+6−2 providers/google.go+18−10 providers/google_test.go+4−4
CHANGELOG.md+1−1
@@ -7,7 +7,7 @@
77- [#905](https://github.com/oauth2-proxy/oauth2-proxy/pull/905) Existing sessions from v6.0.0 or earlier are no longer valid. They will trigger a reauthentication.
88- [#826](https://github.com/oauth2-proxy/oauth2-proxy/pull/826) `skip-auth-strip-headers` now applies to all requests, not just those where authentication would be skipped.
99- [#797](https://github.com/oauth2-proxy/oauth2-proxy/pull/797) The behavior of the Google provider Groups restriction changes with this
10− - Either `--google-group` or the new `--allowed-group` will work for Google now (`--google-group` will be used it both are set)
10+ - Either `--google-group` or the new `--allowed-group` will work for Google now (`--google-group` will be used if both are set)
1111 - Group membership lists will be passed to the backend with the `X-Forwarded-Groups` header
1212 - If you change the list of allowed groups, existing sessions that now don't have a valid group will be logged out immediately.
1313 - Previously, group membership was only checked on session creation and refresh.
oauthproxy.go+6−2
@@ -907,11 +907,15 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) {
907907 }
908908
909909 // set cookie, or deny
910− if p.Validator(session.Email) {
910+ authorized, err := p.provider.Authorize(req.Context(), session)
911+ if err != nil {
912+ logger.Errorf("Error with authorization: %v", err)
913+ }
914+ if p.Validator(session.Email) && authorized {
911915 logger.PrintAuthf(session.Email, req, logger.AuthSuccess, "Authenticated via OAuth2: %s", session)
912916 err := p.SaveSession(rw, req, session)
913917 if err != nil {
914− logger.Printf("Error saving session state for %s: %v", remoteAddr, err)
918+ logger.Errorf("Error saving session state for %s: %v", remoteAddr, err)
915919 p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", err.Error())
916920 return
917921 }
providers/google.go+18−10
@@ -28,13 +28,13 @@ type GoogleProvider struct {
2828
2929 RedeemRefreshURL *url.URL
3030
31− // GroupValidator is a function that determines if the user in the passed
31+ // groupValidator is a function that determines if the user in the passed
3232 // session is a member of any of the configured Google groups.
3333 //
3434 // This hits the Google API for each group, so it is called on Redeem &
3535 // Refresh. `Authorize` uses the results of this saved in `session.Groups`
3636 // Since it is called on every request.
37− GroupValidator func(*sessions.SessionState) bool
37+ groupValidator func(*sessions.SessionState) bool
3838}
3939
4040var _ Provider = (*GoogleProvider)(nil)
@@ -90,9 +90,9 @@ func NewGoogleProvider(p *ProviderData) *GoogleProvider {
9090 })
9191 return &GoogleProvider{
9292 ProviderData: p,
93− // Set a default GroupValidator to just always return valid (true), it will
93+ // Set a default groupValidator to just always return valid (true), it will
9494 // be overwritten if we configured a Google group restriction.
95− GroupValidator: func(*sessions.SessionState) bool {
95+ groupValidator: func(*sessions.SessionState) bool {
9696 return true
9797 },
9898 }
@@ -165,7 +165,8 @@ func (p *GoogleProvider) Redeem(ctx context.Context, redirectURL, code string) (
165165
166166 created := time.Now()
167167 expires := time.Now().Add(time.Duration(jsonResponse.ExpiresIn) * time.Second).Truncate(time.Second)
168− s := &sessions.SessionState{
168+
169+ return &sessions.SessionState{
169170 AccessToken: jsonResponse.AccessToken,
170171 IDToken: jsonResponse.IDToken,
171172 CreatedAt: &created,
@@ -173,19 +174,26 @@ func (p *GoogleProvider) Redeem(ctx context.Context, redirectURL, code string) (
173174 RefreshToken: jsonResponse.RefreshToken,
174175 Email: c.Email,
175176 User: c.Subject,
176− }
177− p.GroupValidator(s)
177+ }, nil
178+}
179+
180+// EnrichSessionState checks the listed Google Groups configured and adds any
181+// that the user is a member of to session.Groups.
182+func (p *GoogleProvider) EnrichSessionState(ctx context.Context, s *sessions.SessionState) error {
183+ p.groupValidator(s)
178184
179− return s, nil
185+ return nil
180186}
181187
182188// SetGroupRestriction configures the GoogleProvider to restrict access to the
183189// specified group(s). AdminEmail has to be an administrative email on the domain that is
184190// checked. CredentialsFile is the path to a json file containing a Google service
185191// account credentials.
192+//
193+// TODO (@NickMeves) - Unit Test this OR refactor away from groupValidator func
186194func (p *GoogleProvider) SetGroupRestriction(groups []string, adminEmail string, credentialsReader io.Reader) {
187195 adminService := getAdminService(adminEmail, credentialsReader)
188− p.GroupValidator = func(s *sessions.SessionState) bool {
196+ p.groupValidator = func(s *sessions.SessionState) bool {
189197 // Reset our saved Groups in case membership changed
190198 // This is used by `Authorize` on every request
191199 s.Groups = make([]string, 0, len(groups))
@@ -266,7 +274,7 @@ func (p *GoogleProvider) RefreshSessionIfNeeded(ctx context.Context, s *sessions
266274 }
267275
268276 // re-check that the user is in the proper google group(s)
269− if !p.GroupValidator(s) {
277+ if !p.groupValidator(s) {
270278 return false, fmt.Errorf("%s is no longer in the group(s)", s.Email)
271279 }
272280
providers/google_test.go+4−4
@@ -118,7 +118,7 @@ func TestGoogleProviderGroupValidator(t *testing.T) {
118118 validatorFunc func(*sessions.SessionState) bool
119119 expectedAuthZ bool
120120 }{
121− "Email is authorized with GroupValidator": {
121+ "Email is authorized with groupValidator": {
122122 session: &sessions.SessionState{
123123 Email: sessionEmail,
124124 },
@@ -127,7 +127,7 @@ func TestGoogleProviderGroupValidator(t *testing.T) {
127127 },
128128 expectedAuthZ: true,
129129 },
130− "Email is denied with GroupValidator": {
130+ "Email is denied with groupValidator": {
131131 session: &sessions.SessionState{
132132 Email: sessionEmail,
133133 },
@@ -149,9 +149,9 @@ func TestGoogleProviderGroupValidator(t *testing.T) {
149149 g := NewWithT(t)
150150 p := newGoogleProvider()
151151 if tc.validatorFunc != nil {
152− p.GroupValidator = tc.validatorFunc
152+ p.groupValidator = tc.validatorFunc
153153 }
154− g.Expect(p.GroupValidator(tc.session)).To(Equal(tc.expectedAuthZ))
154+ g.Expect(p.groupValidator(tc.session)).To(Equal(tc.expectedAuthZ))
155155 })
156156 }
157157}