f21b3b8 · typeable/oauth2-proxy

git.iko.soy/typeable/oauth2-proxy

Authorize in Redeem callback flow

f21b3b8Nick Meves6 years ago

CHANGELOG.md+1−1

@@ -7,7 +7,7 @@

7- [#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.

8- [#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.

9- [#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)

11 - Group membership lists will be passed to the backend with the `X-Forwarded-Groups` header

12 - If you change the list of allowed groups, existing sessions that now don't have a valid group will be logged out immediately.

13 - 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) {

907 }

908

909 // set cookie, or deny

910+ authorized, err := p.provider.Authorize(req.Context(), session)

911+ if err != nil {

912+ logger.Errorf("Error with authorization: %v", err)

913+ }

910− if p.Validator(session.Email) {

914+ if p.Validator(session.Email) && authorized {

911 logger.PrintAuthf(session.Email, req, logger.AuthSuccess, "Authenticated via OAuth2: %s", session)

915 logger.PrintAuthf(session.Email, req, logger.AuthSuccess, "Authenticated via OAuth2: %s", session)

912 err := p.SaveSession(rw, req, session)

916 err := p.SaveSession(rw, req, session)

913 if err != nil {

917 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)

915 p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", err.Error())

919 p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", err.Error())

916 return

920 return

917 }

921 }

providers/google.go+18−10

@@ -28,13 +28,13 @@ type GoogleProvider struct {

29 RedeemRefreshURL *url.URL

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

32 // session is a member of any of the configured Google groups.

33 //

34 // This hits the Google API for each group, so it is called on Redeem &

35 // Refresh. `Authorize` uses the results of this saved in `session.Groups`

36 // Since it is called on every request.

37− GroupValidator func(*sessions.SessionState) bool

37+ groupValidator func(*sessions.SessionState) bool

38}

40var _ Provider = (*GoogleProvider)(nil)

@@ -90,9 +90,9 @@ func NewGoogleProvider(p *ProviderData) *GoogleProvider {

90 })

91 return &GoogleProvider{

92 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

94 // be overwritten if we configured a Google group restriction.

95− GroupValidator: func(*sessions.SessionState) bool {

95+ groupValidator: func(*sessions.SessionState) bool {

96 return true

97 },

98 }

@@ -165,7 +165,8 @@ func (p *GoogleProvider) Redeem(ctx context.Context, redirectURL, code string) (

165

166 created := time.Now()

167 expires := time.Now().Add(time.Duration(jsonResponse.ExpiresIn) * time.Second).Truncate(time.Second)

168+

168− s := &sessions.SessionState{

169+ return &sessions.SessionState{

169 AccessToken: jsonResponse.AccessToken,

170 AccessToken: jsonResponse.AccessToken,

170 IDToken: jsonResponse.IDToken,

171 IDToken: jsonResponse.IDToken,

171 CreatedAt: &created,

172 CreatedAt: &created,

@@ -173,19 +174,26 @@ func (p *GoogleProvider) Redeem(ctx context.Context, redirectURL, code string) (

173 RefreshToken: jsonResponse.RefreshToken,

174 RefreshToken: jsonResponse.RefreshToken,

174 Email: c.Email,

175 Email: c.Email,

175 User: c.Subject,

176 User: c.Subject,

176− }

177+ }, nil

177− p.GroupValidator(s)

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)

178

184

179− return s, nil

185+ return nil

180}

186}

181

187

182// SetGroupRestriction configures the GoogleProvider to restrict access to the

188// SetGroupRestriction configures the GoogleProvider to restrict access to the

183// specified group(s). AdminEmail has to be an administrative email on the domain that is

189// specified group(s). AdminEmail has to be an administrative email on the domain that is

184// checked. CredentialsFile is the path to a json file containing a Google service

190// checked. CredentialsFile is the path to a json file containing a Google service

185// account credentials.

191// account credentials.

192+//

193+// TODO (@NickMeves) - Unit Test this OR refactor away from groupValidator func

186func (p *GoogleProvider) SetGroupRestriction(groups []string, adminEmail string, credentialsReader io.Reader) {

194func (p *GoogleProvider) SetGroupRestriction(groups []string, adminEmail string, credentialsReader io.Reader) {

187 adminService := getAdminService(adminEmail, credentialsReader)

195 adminService := getAdminService(adminEmail, credentialsReader)

188− p.GroupValidator = func(s *sessions.SessionState) bool {

196+ p.groupValidator = func(s *sessions.SessionState) bool {

189 // Reset our saved Groups in case membership changed

197 // Reset our saved Groups in case membership changed

190 // This is used by `Authorize` on every request

198 // This is used by `Authorize` on every request

191 s.Groups = make([]string, 0, len(groups))

199 s.Groups = make([]string, 0, len(groups))

@@ -266,7 +274,7 @@ func (p *GoogleProvider) RefreshSessionIfNeeded(ctx context.Context, s *sessions

266 }

274 }

267

275

268 // re-check that the user is in the proper google group(s)

276 // re-check that the user is in the proper google group(s)

269− if !p.GroupValidator(s) {

277+ if !p.groupValidator(s) {

270 return false, fmt.Errorf("%s is no longer in the group(s)", s.Email)

278 return false, fmt.Errorf("%s is no longer in the group(s)", s.Email)

271 }

279 }

272

280

providers/google_test.go+4−4

@@ -118,7 +118,7 @@ func TestGoogleProviderGroupValidator(t *testing.T) {

118 validatorFunc func(*sessions.SessionState) bool

119 expectedAuthZ bool

120 }{

121− "Email is authorized with GroupValidator": {

121+ "Email is authorized with groupValidator": {

122 session: &sessions.SessionState{

123 Email: sessionEmail,

124 },

@@ -127,7 +127,7 @@ func TestGoogleProviderGroupValidator(t *testing.T) {

127 },

128 expectedAuthZ: true,

129 },

130− "Email is denied with GroupValidator": {

130+ "Email is denied with groupValidator": {

131 session: &sessions.SessionState{

132 Email: sessionEmail,

133 },

@@ -149,9 +149,9 @@ func TestGoogleProviderGroupValidator(t *testing.T) {

149 g := NewWithT(t)

150 p := newGoogleProvider()

151 if tc.validatorFunc != nil {

152− p.GroupValidator = tc.validatorFunc

152+ p.groupValidator = tc.validatorFunc

153 }

154− g.Expect(p.GroupValidator(tc.session)).To(Equal(tc.expectedAuthZ))

154+ g.Expect(p.groupValidator(tc.session)).To(Equal(tc.expectedAuthZ))

155 })

156 }

157}