2aa04c9 · typeable/oauth2-proxy

git.iko.soy/typeable/oauth2-proxy

on master help sign in

Merge pull request #796 from grnhse/refactor-redeem-code

2aa04c9Joel Speed6 years ago

CHANGELOG.md+1

compare with master

unifiedsplit

@@ -26,6 +26,7 @@

26## Changes since v6.1.1

28- [#825](https://github.com/oauth2-proxy/oauth2-proxy/pull/825) Fix code coverage reporting on GitHub actions(@JoelSpeed)

29+- [#796](https://github.com/oauth2-proxy/oauth2-proxy/pull/796) Deprecate GetUserName & GetEmailAdress for EnrichSessionState (@NickMeves)

29- [#705](https://github.com/oauth2-proxy/oauth2-proxy/pull/705) Add generic Header injectors for upstream request and response headers (@JoelSpeed)

30- [#705](https://github.com/oauth2-proxy/oauth2-proxy/pull/705) Add generic Header injectors for upstream request and response headers (@JoelSpeed)

30- [#753](https://github.com/oauth2-proxy/oauth2-proxy/pull/753) Pass resource parameter in login url (@codablock)

31- [#753](https://github.com/oauth2-proxy/oauth2-proxy/pull/753) Pass resource parameter in login url (@codablock)

31- [#789](https://github.com/oauth2-proxy/oauth2-proxy/pull/789) Add `--skip-auth-route` configuration option for `METHOD=pathRegex` based allowlists (@NickMeves)

32- [#789](https://github.com/oauth2-proxy/oauth2-proxy/pull/789) Add `--skip-auth-route` configuration option for `METHOD=pathRegex` based allowlists (@NickMeves)

oauthproxy.go+18−14

@@ -357,22 +357,19 @@ func (p *OAuthProxy) redeemCode(ctx context.Context, host, code string) (*sessio

357 if err != nil {

358 return nil, err

359 }

360+ return s, nil

361+}

360

362

363+func (p *OAuthProxy) enrichSessionState(ctx context.Context, s *sessionsapi.SessionState) error {

364+ var err error

361 if s.Email == "" {

365 if s.Email == "" {

362 s.Email, err = p.provider.GetEmailAddress(ctx, s)

366 s.Email, err = p.provider.GetEmailAddress(ctx, s)

363− if err != nil && err.Error() != "not implemented" {

367+ if err != nil && !errors.Is(err, providers.ErrNotImplemented) {

364− return nil, err

365− }

366− }

367−

368− if s.User == "" {

369− s.User, err = p.provider.GetUserName(ctx, s)

370− if err != nil && err.Error() != "not implemented" {

371− return nil, err

368+ return err

372 }

369 }

373 }

370 }

374

371

375− return s, nil

372+ return p.provider.EnrichSessionState(ctx, s)

376}

373}

377

374

378// MakeCSRFCookie creates a cookie for CSRF

375// MakeCSRFCookie creates a cookie for CSRF

@@ -829,14 +826,21 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) {

829 return

826 return

830 }

827 }

831

828

829+ err = p.enrichSessionState(req.Context(), session)

830+ if err != nil {

831+ logger.Errorf("Error creating session during OAuth2 callback: %v", err)

832+ p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", "Internal Error")

833+ return

834+ }

835+

832− s := strings.SplitN(req.Form.Get("state"), ":", 2)

836+ state := strings.SplitN(req.Form.Get("state"), ":", 2)

833− if len(s) != 2 {

837+ if len(state) != 2 {

834 logger.Error("Error while parsing OAuth2 state: invalid length")

838 logger.Error("Error while parsing OAuth2 state: invalid length")

835 p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", "Invalid State")

839 p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", "Invalid State")

836 return

840 return

837 }

841 }

838− nonce := s[0]

842+ nonce := state[0]

839− redirect := s[1]

843+ redirect := state[1]

840 c, err := req.Cookie(p.CSRFCookieName)

844 c, err := req.Cookie(p.CSRFCookieName)

841 if err != nil {

845 if err != nil {

842 logger.PrintAuthf(session.Email, req, logger.AuthFailure, "Invalid authentication via OAuth2: unable to obtain CSRF cookie")

846 logger.PrintAuthf(session.Email, req, logger.AuthFailure, "Invalid authentication via OAuth2: unable to obtain CSRF cookie")

oauthproxy_test.go+75−3

@@ -396,14 +396,86 @@ func NewTestProvider(providerURL *url.URL, emailAddress string) *TestProvider {

396 }

397}

398

399−func (tp *TestProvider) GetEmailAddress(ctx context.Context, session *sessions.SessionState) (string, error) {

399+func (tp *TestProvider) GetEmailAddress(_ context.Context, _ *sessions.SessionState) (string, error) {

400 return tp.EmailAddress, nil

401}

402

403−func (tp *TestProvider) ValidateSessionState(ctx context.Context, session *sessions.SessionState) bool {

403+func (tp *TestProvider) ValidateSessionState(_ context.Context, _ *sessions.SessionState) bool {

404 return tp.ValidToken

405}

406

407+func Test_redeemCode(t *testing.T) {

408+ opts := baseTestOptions()

409+ err := validation.Validate(opts)

410+ assert.NoError(t, err)

411+

412+ proxy, err := NewOAuthProxy(opts, func(string) bool { return true })

413+ if err != nil {

414+ t.Fatal(err)

415+ }

416+

417+ _, err = proxy.redeemCode(context.Background(), "www.example.com", "")

418+ assert.Error(t, err)

419+}

420+

421+func Test_enrichSession(t *testing.T) {

422+ const (

423+ sessionUser = "Mr Session"

424+ sessionEmail = "session@example.com"

425+ providerEmail = "provider@example.com"

426+ )

427+

428+ testCases := map[string]struct {

429+ session *sessions.SessionState

430+ expectedUser string

431+ expectedEmail string

432+ }{

433+ "Session already has enrichable fields": {

434+ session: &sessions.SessionState{

435+ User: sessionUser,

436+ Email: sessionEmail,

437+ },

438+ expectedUser: sessionUser,

439+ expectedEmail: sessionEmail,

440+ },

441+ "Session is missing Email and GetEmailAddress is implemented": {

442+ session: &sessions.SessionState{

443+ User: sessionUser,

444+ },

445+ expectedUser: sessionUser,

446+ expectedEmail: providerEmail,

447+ },

448+ "Session is missing User and GetUserName is not implemented": {

449+ session: &sessions.SessionState{

450+ Email: sessionEmail,

451+ },

452+ expectedUser: "",

453+ expectedEmail: sessionEmail,

454+ },

455+ }

456+

457+ for name, tc := range testCases {

458+ t.Run(name, func(t *testing.T) {

459+ opts := baseTestOptions()

460+ err := validation.Validate(opts)

461+ assert.NoError(t, err)

462+

463+ // intentionally set after validation.Validate(opts) since it will clobber

464+ // our TestProvider and call `providers.New` defaulting to `providers.GoogleProvider`

465+ opts.SetProvider(NewTestProvider(&url.URL{Host: "www.example.com"}, providerEmail))

466+ proxy, err := NewOAuthProxy(opts, func(string) bool { return true })

467+ if err != nil {

468+ t.Fatal(err)

469+ }

470+

471+ err = proxy.enrichSessionState(context.Background(), tc.session)

472+ assert.NoError(t, err)

473+ assert.Equal(t, tc.expectedUser, tc.session.User)

474+ assert.Equal(t, tc.expectedEmail, tc.session.Email)

475+ })

476+ }

477+}

478+

407func TestBasicAuthPassword(t *testing.T) {

479func TestBasicAuthPassword(t *testing.T) {

408 providerServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

480 providerServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

409 logger.Printf("%#v", r)

481 logger.Printf("%#v", r)

@@ -1883,7 +1955,7 @@ func TestClearSingleCookie(t *testing.T) {

1883type NoOpKeySet struct {

1955type NoOpKeySet struct {

1884}

1956}

1885

1957

1886−func (NoOpKeySet) VerifySignature(ctx context.Context, jwt string) (payload []byte, err error) {

1958+func (NoOpKeySet) VerifySignature(_ context.Context, jwt string) (payload []byte, err error) {

1887 splitStrings := strings.Split(jwt, ".")

1959 splitStrings := strings.Split(jwt, ".")

1888 payloadString := splitStrings[1]

1960 payloadString := splitStrings[1]

1889 return base64.RawURLEncoding.DecodeString(payloadString)

1961 return base64.RawURLEncoding.DecodeString(payloadString)

pkg/validation/options.go−1

@@ -273,7 +273,6 @@ func parseProviderInfo(o *options.Options, msgs []string) []string {

273 case *providers.GitLabProvider:

274 p.AllowUnverifiedEmail = o.InsecureOIDCAllowUnverifiedEmail

275 p.Groups = o.GitLabGroup

276− p.EmailDomains = o.EmailDomains

277

276

278 if o.GetOIDCVerifier() != nil {

277 if o.GetOIDCVerifier() != nil {

279 p.Verifier = o.GetOIDCVerifier()

278 p.Verifier = o.GetOIDCVerifier()

providers/github.go+31−22

@@ -102,6 +102,20 @@ func (p *GitHubProvider) SetUsers(users []string) {

102 p.Users = users

103}

104

105+// EnrichSessionState updates the User & Email after the initial Redeem

106+func (p *GitHubProvider) EnrichSessionState(ctx context.Context, s *sessions.SessionState) error {

107+ err := p.getEmail(ctx, s)

108+ if err != nil {

109+ return err

110+ }

111+ return p.getUser(ctx, s)

112+}

113+

114+// ValidateSessionState validates the AccessToken

115+func (p *GitHubProvider) ValidateSessionState(ctx context.Context, s *sessions.SessionState) bool {

116+ return validateToken(ctx, p, s.AccessToken, makeGitHubHeader(s.AccessToken))

117+}

118+

105func (p *GitHubProvider) hasOrg(ctx context.Context, accessToken string) (bool, error) {

119func (p *GitHubProvider) hasOrg(ctx context.Context, accessToken string) (bool, error) {

106 // https://developer.github.com/v3/orgs/#list-your-organizations

120 // https://developer.github.com/v3/orgs/#list-your-organizations

107

121

@@ -364,8 +378,8 @@ func (p *GitHubProvider) isCollaborator(ctx context.Context, username, accessTok

364 return true, nil

378 return true, nil

365}

379}

366

380

367−// GetEmailAddress returns the Account email address

381+// getEmail updates the SessionState Email

368−func (p *GitHubProvider) GetEmailAddress(ctx context.Context, s *sessions.SessionState) (string, error) {

382+func (p *GitHubProvider) getEmail(ctx context.Context, s *sessions.SessionState) error {

369

383

370 var emails []struct {

384 var emails []struct {

371 Email string `json:"email"`

385 Email string `json:"email"`

@@ -379,11 +393,11 @@ func (p *GitHubProvider) GetEmailAddress(ctx context.Context, s *sessions.Sessio

379 var err error

393 var err error

380 verifiedUser, err = p.hasUser(ctx, s.AccessToken)

394 verifiedUser, err = p.hasUser(ctx, s.AccessToken)

381 if err != nil {

395 if err != nil {

382− return "", err

396+ return err

383 }

397 }

384 // org and repository options are not configured

398 // org and repository options are not configured

385 if !verifiedUser && p.Org == "" && p.Repo == "" {

399 if !verifiedUser && p.Org == "" && p.Repo == "" {

386− return "", errors.New("missing github user")

400+ return errors.New("missing github user")

387 }

401 }

388 }

402 }

389 // If a user is verified by username options, skip the following restrictions

403 // If a user is verified by username options, skip the following restrictions

@@ -391,16 +405,16 @@ func (p *GitHubProvider) GetEmailAddress(ctx context.Context, s *sessions.Sessio

391 if p.Org != "" {

405 if p.Org != "" {

392 if p.Team != "" {

406 if p.Team != "" {

393 if ok, err := p.hasOrgAndTeam(ctx, s.AccessToken); err != nil || !ok {

407 if ok, err := p.hasOrgAndTeam(ctx, s.AccessToken); err != nil || !ok {

394− return "", err

408+ return err

395 }

409 }

396 } else {

410 } else {

397 if ok, err := p.hasOrg(ctx, s.AccessToken); err != nil || !ok {

411 if ok, err := p.hasOrg(ctx, s.AccessToken); err != nil || !ok {

398− return "", err

412+ return err

399 }

413 }

400 }

414 }

401 } else if p.Repo != "" && p.Token == "" { // If we have a token we'll do the collaborator check in GetUserName

415 } else if p.Repo != "" && p.Token == "" { // If we have a token we'll do the collaborator check in GetUserName

402 if ok, err := p.hasRepo(ctx, s.AccessToken); err != nil || !ok {

416 if ok, err := p.hasRepo(ctx, s.AccessToken); err != nil || !ok {

403− return "", err

417+ return err

404 }

418 }

405 }

419 }

406 }

420 }

@@ -416,24 +430,23 @@ func (p *GitHubProvider) GetEmailAddress(ctx context.Context, s *sessions.Sessio

416 Do().

430 Do().

417 UnmarshalInto(&emails)

431 UnmarshalInto(&emails)

418 if err != nil {

432 if err != nil {

419− return "", err

433+ return err

420 }

434 }

421

435

422− returnEmail := ""

423 for _, email := range emails {

436 for _, email := range emails {

424 if email.Verified {

437 if email.Verified {

425− returnEmail = email.Email

426 if email.Primary {

438 if email.Primary {

439+ s.Email = email.Email

427− return returnEmail, nil

440+ return nil

428 }

441 }

429 }

442 }

430 }

443 }

431

444

432− return returnEmail, nil

445+ return nil

433}

446}

434

447

435−// GetUserName returns the Account user name

448+// getUser updates the SessionState User

436−func (p *GitHubProvider) GetUserName(ctx context.Context, s *sessions.SessionState) (string, error) {

449+func (p *GitHubProvider) getUser(ctx context.Context, s *sessions.SessionState) error {

437 var user struct {

450 var user struct {

438 Login string `json:"login"`

451 Login string `json:"login"`

439 Email string `json:"email"`

452 Email string `json:"email"`

@@ -451,22 +464,18 @@ func (p *GitHubProvider) GetUserName(ctx context.Context, s *sessions.SessionSta

451 Do().

464 Do().

452 UnmarshalInto(&user)

465 UnmarshalInto(&user)

453 if err != nil {

466 if err != nil {

454− return "", err

467+ return err

455 }

468 }

456

469

457 // Now that we have the username we can check collaborator status

470 // Now that we have the username we can check collaborator status

458 if !p.isVerifiedUser(user.Login) && p.Org == "" && p.Repo != "" && p.Token != "" {

471 if !p.isVerifiedUser(user.Login) && p.Org == "" && p.Repo != "" && p.Token != "" {

459 if ok, err := p.isCollaborator(ctx, user.Login, p.Token); err != nil || !ok {

472 if ok, err := p.isCollaborator(ctx, user.Login, p.Token); err != nil || !ok {

460− return "", err

473+ return err

461 }

474 }

462 }

475 }

463

476

464− return user.Login, nil

477+ s.User = user.Login

465−}

478+ return nil

466−

467−// ValidateSessionState validates the AccessToken

468−func (p *GitHubProvider) ValidateSessionState(ctx context.Context, s *sessions.SessionState) bool {

469− return validateToken(ctx, p, s.AccessToken, makeGitHubHeader(s.AccessToken))

470}

479}

471

480

472// isVerifiedUser

481// isVerifiedUser

providers/github_test.go+71−71

@@ -107,7 +107,7 @@ func TestGitHubProviderOverrides(t *testing.T) {

107 assert.Equal(t, "profile", p.Data().Scope)

108}

109

110−func TestGitHubProviderGetEmailAddress(t *testing.T) {

110+func TestGitHubProvider_getEmail(t *testing.T) {

111 b := testGitHubBackend(map[string][]string{

112 "/user/emails": {`[ {"email": "michael.bland@gsa.gov", "verified": true, "primary": true} ]`},

113 })

@@ -117,14 +117,14 @@ func TestGitHubProviderGetEmailAddress(t *testing.T) {

117 p := testGitHubProvider(bURL.Host)

118

119 session := CreateAuthorizedSession()

120− email, err := p.GetEmailAddress(context.Background(), session)

120+ err := p.getEmail(context.Background(), session)

121− assert.Equal(t, nil, err)

121+ assert.NoError(t, err)

122− assert.Equal(t, "michael.bland@gsa.gov", email)

122+ assert.Equal(t, "michael.bland@gsa.gov", session.Email)

123}

124

125−func TestGitHubProviderGetEmailAddressNotVerified(t *testing.T) {

125+func TestGitHubProvider_getEmailNotVerified(t *testing.T) {

126 b := testGitHubBackend(map[string][]string{

127− "/user/emails": {`[ {"email": "michael.bland@gsa.gov", "verified": true, "primary": true} ]`},

127+ "/user/emails": {`[ {"email": "michael.bland@gsa.gov", "verified": false, "primary": true} ]`},

128 })

129 defer b.Close()

130

@@ -132,12 +132,12 @@ func TestGitHubProviderGetEmailAddressNotVerified(t *testing.T) {

132 p := testGitHubProvider(bURL.Host)

133

134 session := CreateAuthorizedSession()

135− email, err := p.GetEmailAddress(context.Background(), session)

135+ err := p.getEmail(context.Background(), session)

136− assert.Equal(t, nil, err)

136+ assert.NoError(t, err)

137− assert.Empty(t, "", email)

137+ assert.Empty(t, session.Email)

138}

139

140−func TestGitHubProviderGetEmailAddressWithOrg(t *testing.T) {

140+func TestGitHubProvider_getEmailWithOrg(t *testing.T) {

141 b := testGitHubBackend(map[string][]string{

142 "/user/emails": {`[ {"email": "michael.bland@gsa.gov", "verified": true, "primary": true} ]`},

143 "/user/orgs": {

@@ -153,12 +153,12 @@ func TestGitHubProviderGetEmailAddressWithOrg(t *testing.T) {

153 p.Org = "testorg1"

154

155 session := CreateAuthorizedSession()

156− email, err := p.GetEmailAddress(context.Background(), session)

156+ err := p.getEmail(context.Background(), session)

157− assert.Equal(t, nil, err)

157+ assert.NoError(t, err)

158− assert.Equal(t, "michael.bland@gsa.gov", email)

158+ assert.Equal(t, "michael.bland@gsa.gov", session.Email)

159}

160

161−func TestGitHubProviderGetEmailAddressWithWriteAccessToPublicRepo(t *testing.T) {

161+func TestGitHubProvider_getEmailWithWriteAccessToPublicRepo(t *testing.T) {

162 b := testGitHubBackend(map[string][]string{

163 "/repo/oauth2-proxy/oauth2-proxy": {`{"permissions": {"pull": true, "push": true}, "private": false}`},

164 "/user/emails": {`[ {"email": "michael.bland@gsa.gov", "verified": true, "primary": true} ]`},

@@ -170,12 +170,12 @@ func TestGitHubProviderGetEmailAddressWithWriteAccessToPublicRepo(t *testing.T)

170 p.SetRepo("oauth2-proxy/oauth2-proxy", "")

171

172 session := CreateAuthorizedSession()

173− email, err := p.GetEmailAddress(context.Background(), session)

173+ err := p.getEmail(context.Background(), session)

174− assert.Equal(t, nil, err)

174+ assert.NoError(t, err)

175− assert.Equal(t, "michael.bland@gsa.gov", email)

175+ assert.Equal(t, "michael.bland@gsa.gov", session.Email)

176}

177

178−func TestGitHubProviderGetEmailAddressWithReadOnlyAccessToPrivateRepo(t *testing.T) {

178+func TestGitHubProvider_getEmailWithReadOnlyAccessToPrivateRepo(t *testing.T) {

179 b := testGitHubBackend(map[string][]string{

180 "/repo/oauth2-proxy/oauth2-proxy": {`{"permissions": {"pull": true, "push": false}, "private": true}`},

181 "/user/emails": {`[ {"email": "michael.bland@gsa.gov", "verified": true, "primary": true} ]`},

@@ -187,12 +187,12 @@ func TestGitHubProviderGetEmailAddressWithReadOnlyAccessToPrivateRepo(t *testing

187 p.SetRepo("oauth2-proxy/oauth2-proxy", "")

188

189 session := CreateAuthorizedSession()

190− email, err := p.GetEmailAddress(context.Background(), session)

190+ err := p.getEmail(context.Background(), session)

191− assert.Equal(t, nil, err)

191+ assert.NoError(t, err)

192− assert.Equal(t, "michael.bland@gsa.gov", email)

192+ assert.Equal(t, "michael.bland@gsa.gov", session.Email)

193}

194

195−func TestGitHubProviderGetEmailAddressWithWriteAccessToPrivateRepo(t *testing.T) {

195+func TestGitHubProvider_getEmailWithWriteAccessToPrivateRepo(t *testing.T) {

196 b := testGitHubBackend(map[string][]string{

197 "/repo/oauth2-proxy/oauth2-proxy": {`{"permissions": {"pull": true, "push": true}, "private": true}`},

198 "/user/emails": {`[ {"email": "michael.bland@gsa.gov", "verified": true, "primary": true} ]`},

@@ -204,14 +204,14 @@ func TestGitHubProviderGetEmailAddressWithWriteAccessToPrivateRepo(t *testing.T)

204 p.SetRepo("oauth2-proxy/oauth2-proxy", "")

205

206 session := CreateAuthorizedSession()

207− email, err := p.GetEmailAddress(context.Background(), session)

207+ err := p.getEmail(context.Background(), session)

208− assert.Equal(t, nil, err)

208+ assert.NoError(t, err)

209− assert.Equal(t, "michael.bland@gsa.gov", email)

209+ assert.Equal(t, "michael.bland@gsa.gov", session.Email)

210}

211

212−func TestGitHubProviderGetEmailAddressWithNoAccessToPrivateRepo(t *testing.T) {

212+func TestGitHubProvider_getEmailWithNoAccessToPrivateRepo(t *testing.T) {

213 b := testGitHubBackend(map[string][]string{

214− "/repo/oauth2-proxy/oauth2-proxy": {},

214+ "/repo/oauth2-proxy/oauth2-proxy": {`{}`},

215 })

216 defer b.Close()

217

@@ -220,12 +220,12 @@ func TestGitHubProviderGetEmailAddressWithNoAccessToPrivateRepo(t *testing.T) {

220 p.SetRepo("oauth2-proxy/oauth2-proxy", "")

221

222 session := CreateAuthorizedSession()

223− email, err := p.GetEmailAddress(context.Background(), session)

223+ err := p.getEmail(context.Background(), session)

224− assert.NotEqual(t, nil, err)

224+ assert.NoError(t, err)

225− assert.Equal(t, "", email)

225+ assert.Empty(t, session.Email)

226}

227

228−func TestGitHubProviderGetEmailAddressWithToken(t *testing.T) {

228+func TestGitHubProvider_getEmailWithToken(t *testing.T) {

229 b := testGitHubBackend(map[string][]string{

230 "/user/emails": {`[ {"email": "michael.bland@gsa.gov", "verified": true, "primary": true} ]`},

231 })

@@ -236,14 +236,14 @@ func TestGitHubProviderGetEmailAddressWithToken(t *testing.T) {

236 p.SetRepo("oauth2-proxy/oauth2-proxy", "token")

237

238 session := CreateAuthorizedSession()

239− email, err := p.GetEmailAddress(context.Background(), session)

239+ err := p.getEmail(context.Background(), session)

240− assert.Equal(t, nil, err)

240+ assert.NoError(t, err)

241− assert.Equal(t, "michael.bland@gsa.gov", email)

241+ assert.Equal(t, "michael.bland@gsa.gov", session.Email)

242}

243

244// Note that trying to trigger the "failed building request" case is not

245// practical, since the only way it can fail is if the URL fails to parse.

246−func TestGitHubProviderGetEmailAddressFailedRequest(t *testing.T) {

246+func TestGitHubProvider_getEmailFailedRequest(t *testing.T) {

247 b := testGitHubBackend(map[string][]string{})

248 defer b.Close()

249

@@ -254,12 +254,12 @@ func TestGitHubProviderGetEmailAddressFailedRequest(t *testing.T) {

254 // token. Alternatively, we could allow the parsing of the payload as

255 // JSON to fail.

256 session := &sessions.SessionState{AccessToken: "unexpected_access_token"}

257− email, err := p.GetEmailAddress(context.Background(), session)

257+ err := p.getEmail(context.Background(), session)

258− assert.NotEqual(t, nil, err)

258+ assert.Error(t, err)

259− assert.Equal(t, "", email)

259+ assert.Empty(t, session.Email)

260}

261

262−func TestGitHubProviderGetEmailAddressEmailNotPresentInPayload(t *testing.T) {

262+func TestGitHubProvider_getEmailNotPresentInPayload(t *testing.T) {

263 b := testGitHubBackend(map[string][]string{

264 "/user/emails": {`{"foo": "bar"}`},

265 })

@@ -269,12 +269,12 @@ func TestGitHubProviderGetEmailAddressEmailNotPresentInPayload(t *testing.T) {

269 p := testGitHubProvider(bURL.Host)

270

271 session := CreateAuthorizedSession()

272− email, err := p.GetEmailAddress(context.Background(), session)

272+ err := p.getEmail(context.Background(), session)

273− assert.NotEqual(t, nil, err)

273+ assert.Error(t, err)

274− assert.Equal(t, "", email)

274+ assert.Empty(t, session.Email)

275}

276

277−func TestGitHubProviderGetUserName(t *testing.T) {

277+func TestGitHubProvider_getUser(t *testing.T) {

278 b := testGitHubBackend(map[string][]string{

279 "/user": {`{"email": "michael.bland@gsa.gov", "login": "mbland"}`},

280 })

@@ -284,12 +284,12 @@ func TestGitHubProviderGetUserName(t *testing.T) {

284 p := testGitHubProvider(bURL.Host)

285

286 session := CreateAuthorizedSession()

287− email, err := p.GetUserName(context.Background(), session)

287+ err := p.getUser(context.Background(), session)

288− assert.Equal(t, nil, err)

288+ assert.NoError(t, err)

289− assert.Equal(t, "mbland", email)

289+ assert.Equal(t, "mbland", session.User)

290}

291

292−func TestGitHubProviderGetUserNameWithRepoAndToken(t *testing.T) {

292+func TestGitHubProvider_getUserWithRepoAndToken(t *testing.T) {

293 b := testGitHubBackend(map[string][]string{

294 "/user": {`{"email": "michael.bland@gsa.gov", "login": "mbland"}`},

295 "/repos/oauth2-proxy/oauth2-proxy/collaborators/mbland": {""},

@@ -301,12 +301,12 @@ func TestGitHubProviderGetUserNameWithRepoAndToken(t *testing.T) {

301 p.SetRepo("oauth2-proxy/oauth2-proxy", "token")

302

303 session := CreateAuthorizedSession()

304− email, err := p.GetUserName(context.Background(), session)

304+ err := p.getUser(context.Background(), session)

305− assert.Equal(t, nil, err)

305+ assert.NoError(t, err)

306− assert.Equal(t, "mbland", email)

306+ assert.Equal(t, "mbland", session.User)

307}

308

309−func TestGitHubProviderGetUserNameWithRepoAndTokenWithoutPushAccess(t *testing.T) {

309+func TestGitHubProvider_getUserWithRepoAndTokenWithoutPushAccess(t *testing.T) {

310 b := testGitHubBackend(map[string][]string{})

311 defer b.Close()

312

@@ -315,12 +315,12 @@ func TestGitHubProviderGetUserNameWithRepoAndTokenWithoutPushAccess(t *testing.T

315 p.SetRepo("oauth2-proxy/oauth2-proxy", "token")

316

317 session := CreateAuthorizedSession()

318− email, err := p.GetUserName(context.Background(), session)

318+ err := p.getUser(context.Background(), session)

319− assert.NotEqual(t, nil, err)

319+ assert.Error(t, err)

320− assert.Equal(t, "", email)

320+ assert.Empty(t, session.User)

321}

322

323−func TestGitHubProviderGetEmailAddressWithUsername(t *testing.T) {

323+func TestGitHubProvider_getEmailWithUsername(t *testing.T) {

324 b := testGitHubBackend(map[string][]string{

325 "/user": {`{"email": "michael.bland@gsa.gov", "login": "mbland"}`},

326 "/user/emails": {`[ {"email": "michael.bland@gsa.gov", "verified": true, "primary": true} ]`},

@@ -332,12 +332,12 @@ func TestGitHubProviderGetEmailAddressWithUsername(t *testing.T) {

332 p.SetUsers([]string{"mbland", "octocat"})

333

334 session := CreateAuthorizedSession()

335− email, err := p.GetEmailAddress(context.Background(), session)

335+ err := p.getEmail(context.Background(), session)

336− assert.Equal(t, nil, err)

336+ assert.NoError(t, err)

337− assert.Equal(t, "michael.bland@gsa.gov", email)

337+ assert.Equal(t, "michael.bland@gsa.gov", session.Email)

338}

339

340−func TestGitHubProviderGetEmailAddressWithNotAllowedUsername(t *testing.T) {

340+func TestGitHubProvider_getEmailWithNotAllowedUsername(t *testing.T) {

341 b := testGitHubBackend(map[string][]string{

342 "/user": {`{"email": "michael.bland@gsa.gov", "login": "mbland"}`},

343 "/user/emails": {`[ {"email": "michael.bland@gsa.gov", "verified": true, "primary": true} ]`},

@@ -349,12 +349,12 @@ func TestGitHubProviderGetEmailAddressWithNotAllowedUsername(t *testing.T) {

349 p.SetUsers([]string{"octocat"})

350

351 session := CreateAuthorizedSession()

352− email, err := p.GetEmailAddress(context.Background(), session)

352+ err := p.getEmail(context.Background(), session)

353− assert.NotEqual(t, nil, err)

353+ assert.Error(t, err)

354− assert.Equal(t, "", email)

354+ assert.Empty(t, session.Email)

355}

356

357−func TestGitHubProviderGetEmailAddressWithUsernameAndNotBelongToOrg(t *testing.T) {

357+func TestGitHubProvider_getEmailWithUsernameAndNotBelongToOrg(t *testing.T) {

358 b := testGitHubBackend(map[string][]string{

359 "/user": {`{"email": "michael.bland@gsa.gov", "login": "mbland"}`},

360 "/user/emails": {`[ {"email": "michael.bland@gsa.gov", "verified": true, "primary": true} ]`},

@@ -371,16 +371,16 @@ func TestGitHubProviderGetEmailAddressWithUsernameAndNotBelongToOrg(t *testing.T

371 p.SetUsers([]string{"mbland"})

372

373 session := CreateAuthorizedSession()

374− email, err := p.GetEmailAddress(context.Background(), session)

374+ err := p.getEmail(context.Background(), session)

375− assert.Equal(t, nil, err)

375+ assert.NoError(t, err)

376− assert.Equal(t, "michael.bland@gsa.gov", email)

376+ assert.Equal(t, "michael.bland@gsa.gov", session.Email)

377}

378

379−func TestGitHubProviderGetEmailAddressWithUsernameAndNoAccessToPrivateRepo(t *testing.T) {

379+func TestGitHubProvider_getEmailWithUsernameAndNoAccessToPrivateRepo(t *testing.T) {

380 b := testGitHubBackend(map[string][]string{

381 "/user": {`{"email": "michael.bland@gsa.gov", "login": "mbland"}`},

382 "/user/emails": {`[ {"email": "michael.bland@gsa.gov", "verified": true, "primary": true} ]`},

383− "/repo/oauth2-proxy/oauth2-proxy": {},

383+ "/repo/oauth2-proxy/oauth2-proxy": {`{}`},

384 })

385 defer b.Close()

386

@@ -390,7 +390,7 @@ func TestGitHubProviderGetEmailAddressWithUsernameAndNoAccessToPrivateRepo(t *te

390 p.SetUsers([]string{"mbland"})

391

392 session := CreateAuthorizedSession()

393− email, err := p.GetEmailAddress(context.Background(), session)

393+ err := p.getEmail(context.Background(), session)

394− assert.Equal(t, nil, err)

394+ assert.NoError(t, err)

395− assert.Equal(t, "michael.bland@gsa.gov", email)

395+ assert.Equal(t, "michael.bland@gsa.gov", session.Email)

396}

providers/gitlab.go+9−38

@@ -3,7 +3,6 @@ package providers

3import (

4 "context"

5 "fmt"

6− "strings"

7 "time"

6 "time"

9 oidc "github.com/coreos/go-oidc"

8 oidc "github.com/coreos/go-oidc"

@@ -16,9 +15,7 @@ import (

16type GitLabProvider struct {

15type GitLabProvider struct {

17 *ProviderData

16 *ProviderData

19− Groups []string

18+ Groups []string

20− EmailDomains []string

21−

22 Verifier *oidc.IDTokenVerifier

19 Verifier *oidc.IDTokenVerifier

23 AllowUnverifiedEmail bool

20 AllowUnverifiedEmail bool

24}

21}

@@ -168,20 +165,6 @@ func (p *GitLabProvider) verifyGroupMembership(userInfo *gitlabUserInfo) error {

168 return fmt.Errorf("user is not a member of '%s'", p.Groups)

165 return fmt.Errorf("user is not a member of '%s'", p.Groups)

169}

166}

170

167

171−func (p *GitLabProvider) verifyEmailDomain(userInfo *gitlabUserInfo) error {

172− if len(p.EmailDomains) == 0 || p.EmailDomains[0] == "*" {

173− return nil

174− }

175−

176− for _, domain := range p.EmailDomains {

177− if strings.HasSuffix(userInfo.Email, domain) {

178− return nil

179− }

180− }

181−

182− return fmt.Errorf("user email is not one of the valid domains '%v'", p.EmailDomains)

183−}

184−

185func (p *GitLabProvider) createSessionState(ctx context.Context, token *oauth2.Token) (*sessions.SessionState, error) {

168func (p *GitLabProvider) createSessionState(ctx context.Context, token *oauth2.Token) (*sessions.SessionState, error) {

186 rawIDToken, ok := token.Extra("id_token").(string)

169 rawIDToken, ok := token.Extra("id_token").(string)

187 if !ok {

170 if !ok {

@@ -211,39 +194,27 @@ func (p *GitLabProvider) ValidateSessionState(ctx context.Context, s *sessions.S

211}

194}

212

195

213// GetEmailAddress returns the Account email address

196// GetEmailAddress returns the Account email address

214−func (p *GitLabProvider) GetEmailAddress(ctx context.Context, s *sessions.SessionState) (string, error) {

197+func (p *GitLabProvider) EnrichSessionState(ctx context.Context, s *sessions.SessionState) error {

215 // Retrieve user info

198 // Retrieve user info

216 userInfo, err := p.getUserInfo(ctx, s)

199 userInfo, err := p.getUserInfo(ctx, s)

217 if err != nil {

200 if err != nil {

218− return "", fmt.Errorf("failed to retrieve user info: %v", err)

201+ return fmt.Errorf("failed to retrieve user info: %v", err)

219 }

202 }

220

203

221 // Check if email is verified

204 // Check if email is verified

222 if !p.AllowUnverifiedEmail && !userInfo.EmailVerified {

205 if !p.AllowUnverifiedEmail && !userInfo.EmailVerified {

223− return "", fmt.Errorf("user email is not verified")

206+ return fmt.Errorf("user email is not verified")

224− }

225−

226− // Check if email has valid domain

227− err = p.verifyEmailDomain(userInfo)

228− if err != nil {

229− return "", fmt.Errorf("email domain check failed: %v", err)

230 }

207 }

231

208

232 // Check group membership

209 // Check group membership

210+ // TODO (@NickMeves) - Refactor to Authorize

233 err = p.verifyGroupMembership(userInfo)

211 err = p.verifyGroupMembership(userInfo)

234 if err != nil {

212 if err != nil {

235− return "", fmt.Errorf("group membership check failed: %v", err)

213+ return fmt.Errorf("group membership check failed: %v", err)

236 }

214 }

237

215

238− return userInfo.Email, nil

216+ s.User = userInfo.Username

239−}

217+ s.Email = userInfo.Email

240−

241−// GetUserName returns the Account user name

242−func (p *GitLabProvider) GetUserName(ctx context.Context, s *sessions.SessionState) (string, error) {

243− userInfo, err := p.getUserInfo(ctx, s)

244− if err != nil {

245− return "", fmt.Errorf("failed to retrieve user info: %v", err)

246− }

247

218

248− return userInfo.Username, nil

219+ return nil

249}

220}

providers/gitlab_test.go+15−44

@@ -64,8 +64,8 @@ func TestGitLabProviderBadToken(t *testing.T) {

64 p := testGitLabProvider(bURL.Host)

66 session := &sessions.SessionState{AccessToken: "unexpected_gitlab_access_token"}

67− _, err := p.GetEmailAddress(context.Background(), session)

67+ err := p.EnrichSessionState(context.Background(), session)

68− assert.NotEqual(t, nil, err)

68+ assert.Error(t, err)

69}

71func TestGitLabProviderUnverifiedEmailDenied(t *testing.T) {

@@ -76,8 +76,8 @@ func TestGitLabProviderUnverifiedEmailDenied(t *testing.T) {

76 p := testGitLabProvider(bURL.Host)

78 session := &sessions.SessionState{AccessToken: "gitlab_access_token"}

79− _, err := p.GetEmailAddress(context.Background(), session)

79+ err := p.EnrichSessionState(context.Background(), session)

80− assert.NotEqual(t, nil, err)

80+ assert.Error(t, err)

81}

83func TestGitLabProviderUnverifiedEmailAllowed(t *testing.T) {

@@ -89,9 +89,9 @@ func TestGitLabProviderUnverifiedEmailAllowed(t *testing.T) {

89 p.AllowUnverifiedEmail = true

91 session := &sessions.SessionState{AccessToken: "gitlab_access_token"}

92− email, err := p.GetEmailAddress(context.Background(), session)

92+ err := p.EnrichSessionState(context.Background(), session)

93− assert.Equal(t, nil, err)

93+ assert.NoError(t, err)

94− assert.Equal(t, "foo@bar.com", email)

94+ assert.Equal(t, "foo@bar.com", session.Email)

95}

97func TestGitLabProviderUsername(t *testing.T) {

@@ -103,9 +103,9 @@ func TestGitLabProviderUsername(t *testing.T) {

103 p.AllowUnverifiedEmail = true

104

105 session := &sessions.SessionState{AccessToken: "gitlab_access_token"}

106− username, err := p.GetUserName(context.Background(), session)

106+ err := p.EnrichSessionState(context.Background(), session)

107− assert.Equal(t, nil, err)

107+ assert.NoError(t, err)

108− assert.Equal(t, "FooBar", username)

108+ assert.Equal(t, "FooBar", session.User)

109}

110

111func TestGitLabProviderGroupMembershipValid(t *testing.T) {

@@ -118,9 +118,9 @@ func TestGitLabProviderGroupMembershipValid(t *testing.T) {

118 p.Groups = []string{"foo"}

119

120 session := &sessions.SessionState{AccessToken: "gitlab_access_token"}

121− email, err := p.GetEmailAddress(context.Background(), session)

121+ err := p.EnrichSessionState(context.Background(), session)

122− assert.Equal(t, nil, err)

122+ assert.NoError(t, err)

123− assert.Equal(t, "foo@bar.com", email)

123+ assert.Equal(t, "FooBar", session.User)

124}

125

126func TestGitLabProviderGroupMembershipMissing(t *testing.T) {

@@ -133,35 +133,6 @@ func TestGitLabProviderGroupMembershipMissing(t *testing.T) {

133 p.Groups = []string{"baz"}

134

135 session := &sessions.SessionState{AccessToken: "gitlab_access_token"}

136− _, err := p.GetEmailAddress(context.Background(), session)

136+ err := p.EnrichSessionState(context.Background(), session)

137− assert.NotEqual(t, nil, err)

137+ assert.Error(t, err)

138−}

139−

140−func TestGitLabProviderEmailDomainValid(t *testing.T) {

141− b := testGitLabBackend()

142− defer b.Close()

143−

144− bURL, _ := url.Parse(b.URL)

145− p := testGitLabProvider(bURL.Host)

146− p.AllowUnverifiedEmail = true

147− p.EmailDomains = []string{"bar.com"}

148−

149− session := &sessions.SessionState{AccessToken: "gitlab_access_token"}

150− email, err := p.GetEmailAddress(context.Background(), session)

151− assert.Equal(t, nil, err)

152− assert.Equal(t, "foo@bar.com", email)

153−}

154−

155−func TestGitLabProviderEmailDomainInvalid(t *testing.T) {

156− b := testGitLabBackend()

157− defer b.Close()

158−

159− bURL, _ := url.Parse(b.URL)

160− p := testGitLabProvider(bURL.Host)

161− p.AllowUnverifiedEmail = true

162− p.EmailDomains = []string{"baz.com"}

163−

164− session := &sessions.SessionState{AccessToken: "gitlab_access_token"}

165− _, err := p.GetEmailAddress(context.Background(), session)

166− assert.NotEqual(t, nil, err)

167}

138}

providers/provider_default.go+20−12

@@ -14,7 +14,13 @@ import (

14 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/requests"

15)

17+var (

18+ // ErrNotImplemented is returned when a provider did not override a default

19+ // implementation method that doesn't have sensible defaults

20+ ErrNotImplemented = errors.New("not implemented")

21+

17−var _ Provider = (*ProviderData)(nil)

22+ _ Provider = (*ProviderData)(nil)

23+)

19// Redeem provides a default implementation of the OAuth2 token redemption process

25// Redeem provides a default implementation of the OAuth2 token redemption process

20func (p *ProviderData) Redeem(ctx context.Context, redirectURL, code string) (s *sessions.SessionState, err error) {

26func (p *ProviderData) Redeem(ctx context.Context, redirectURL, code string) (s *sessions.SessionState, err error) {

@@ -81,21 +87,23 @@ func (p *ProviderData) GetLoginURL(redirectURI, state string) string {

81}

87}

83// GetEmailAddress returns the Account email address

89// GetEmailAddress returns the Account email address

84−func (p *ProviderData) GetEmailAddress(ctx context.Context, s *sessions.SessionState) (string, error) {

90+// DEPRECATED: Migrate to EnrichSessionState

85− return "", errors.New("not implemented")

86−}

87−

88−// GetUserName returns the Account username

89−func (p *ProviderData) GetUserName(ctx context.Context, s *sessions.SessionState) (string, error) {

91+func (p *ProviderData) GetEmailAddress(_ context.Context, _ *sessions.SessionState) (string, error) {

90− return "", errors.New("not implemented")

92+ return "", ErrNotImplemented

91}

93}

93// ValidateGroup validates that the provided email exists in the configured provider

95// ValidateGroup validates that the provided email exists in the configured provider

94// email group(s).

96// email group(s).

95−func (p *ProviderData) ValidateGroup(email string) bool {

97+func (p *ProviderData) ValidateGroup(_ string) bool {

96 return true

98 return true

97}

99}

100

101+// EnrichSessionState is called after Redeem to allow providers to enrich session fields

102+// such as User, Email, Groups with provider specific API calls.

103+func (p *ProviderData) EnrichSessionState(_ context.Context, _ *sessions.SessionState) error {

104+ return nil

105+}

106+

99// ValidateSessionState validates the AccessToken

107// ValidateSessionState validates the AccessToken

100func (p *ProviderData) ValidateSessionState(ctx context.Context, s *sessions.SessionState) bool {

108func (p *ProviderData) ValidateSessionState(ctx context.Context, s *sessions.SessionState) bool {

101 return validateToken(ctx, p, s.AccessToken, nil)

109 return validateToken(ctx, p, s.AccessToken, nil)

@@ -103,12 +111,12 @@ func (p *ProviderData) ValidateSessionState(ctx context.Context, s *sessions.Ses

103

111

104// RefreshSessionIfNeeded should refresh the user's session if required and

112// RefreshSessionIfNeeded should refresh the user's session if required and

105// do nothing if a refresh is not required

113// do nothing if a refresh is not required

106−func (p *ProviderData) RefreshSessionIfNeeded(ctx context.Context, s *sessions.SessionState) (bool, error) {

114+func (p *ProviderData) RefreshSessionIfNeeded(_ context.Context, _ *sessions.SessionState) (bool, error) {

107 return false, nil

115 return false, nil

108}

116}

109

117

110// CreateSessionStateFromBearerToken should be implemented to allow providers

118// CreateSessionStateFromBearerToken should be implemented to allow providers

111// to convert ID tokens into sessions

119// to convert ID tokens into sessions

112−func (p *ProviderData) CreateSessionStateFromBearerToken(ctx context.Context, rawIDToken string, idToken *oidc.IDToken) (*sessions.SessionState, error) {

120+func (p *ProviderData) CreateSessionStateFromBearerToken(_ context.Context, _ string, _ *oidc.IDToken) (*sessions.SessionState, error) {

113− return nil, errors.New("not implemented")

121+ return nil, ErrNotImplemented

114}

122}

providers/provider_default_test.go+6

@@ -47,3 +47,9 @@ func TestAcrValuesConfigured(t *testing.T) {

47 result := p.GetLoginURL("https://my.test.app/oauth", "")

48 assert.Contains(t, result, "acr_values=testValue")

49}

50+

51+func TestEnrichSessionState(t *testing.T) {

52+ p := &ProviderData{}

53+ s := &sessions.SessionState{}

54+ assert.NoError(t, p.EnrichSessionState(context.Background(), s))

55+}

providers/providers.go+2−1

@@ -10,10 +10,11 @@ import (

10// Provider represents an upstream identity provider implementation

11type Provider interface {

12 Data() *ProviderData

13+ // DEPRECATED: Migrate to EnrichSessionState

13 GetEmailAddress(ctx context.Context, s *sessions.SessionState) (string, error)

14 GetEmailAddress(ctx context.Context, s *sessions.SessionState) (string, error)

14− GetUserName(ctx context.Context, s *sessions.SessionState) (string, error)

15 Redeem(ctx context.Context, redirectURI, code string) (*sessions.SessionState, error)

16 ValidateGroup(string) bool

17+ EnrichSessionState(ctx context.Context, s *sessions.SessionState) error

17 ValidateSessionState(ctx context.Context, s *sessions.SessionState) bool

18 ValidateSessionState(ctx context.Context, s *sessions.SessionState) bool

18 GetLoginURL(redirectURI, finalRedirect string) string

19 GetLoginURL(redirectURI, finalRedirect string) string

19 RefreshSessionIfNeeded(ctx context.Context, s *sessions.SessionState) (bool, error)

20 RefreshSessionIfNeeded(ctx context.Context, s *sessions.SessionState) (bool, error)