e51f5fe · typeable/oauth2-proxy

git.iko.soy/typeable/oauth2-proxy

Refactor GitHub to EnrichSessionState

commite51f5fe7c9b939a06abad4f32cd846322a40e068 authorNick Meves <nick.meves@greenhouse.io> dateSun 27 Sep 2020, 13:35 (6 years ago) parent2b9e1bb

2 files changed+102−93

compare with master

unifiedsplit

providers/github.go+31−22 providers/github_test.go+71−71

providers/github.go+31−22

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

102102 p.Users = users

103103}

104104

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+

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

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

107121

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

364378 return true, nil

365379}

366380

367−// GetEmailAddress returns the Account email address

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

381+// getEmail updates the SessionState Email

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

369383

370384 var emails []struct {

371385 Email string `json:"email"`

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

379393 var err error

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

381395 if err != nil {

382− return "", err

396+ return err

383397 }

384398 // org and repository options are not configured

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

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

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

387401 }

388402 }

389403 // 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

391405 if p.Org != "" {

392406 if p.Team != "" {

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

394− return "", err

408+ return err

395409 }

396410 } else {

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

398− return "", err

412+ return err

399413 }

400414 }

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

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

403− return "", err

417+ return err

404418 }

405419 }

406420 }

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

416430 Do().

417431 UnmarshalInto(&emails)

418432 if err != nil {

419− return "", err

433+ return err

420434 }

421435

422− returnEmail := ""

423436 for _, email := range emails {

424437 if email.Verified {

425− returnEmail = email.Email

426438 if email.Primary {

427− return returnEmail, nil

439+ s.Email = email.Email

440+ return nil

428441 }

429442 }

430443 }

431444

432− return returnEmail, nil

445+ return nil

433446}

434447

435−// GetUserName returns the Account user name

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

448+// getUser updates the SessionState User

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

437450 var user struct {

438451 Login string `json:"login"`

439452 Email string `json:"email"`

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

451464 Do().

452465 UnmarshalInto(&user)

453466 if err != nil {

454− return "", err

467+ return err

455468 }

456469

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

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

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

460− return "", err

473+ return err

461474 }

462475 }

463476

464− return user.Login, nil

465−}

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

477+ s.User = user.Login

478+ return nil

470479}

471480

472481// isVerifiedUser

providers/github_test.go+71−71

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

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

108108}

109109

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

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

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

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

113113 })

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

117117 p := testGitHubProvider(bURL.Host)

118118

119119 session := CreateAuthorizedSession()

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

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

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

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

121+ assert.NoError(t, err)

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

123123}

124124

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

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

126126 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} ]`},

128128 })

129129 defer b.Close()

130130

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

132132 p := testGitHubProvider(bURL.Host)

133133

134134 session := CreateAuthorizedSession()

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

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

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

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

136+ assert.NoError(t, err)

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

138138}

139139

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

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

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

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

143143 "/user/orgs": {

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

153153 p.Org = "testorg1"

154154

155155 session := CreateAuthorizedSession()

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

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

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

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

157+ assert.NoError(t, err)

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

159159}

160160

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

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

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

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

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

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

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

171171

172172 session := CreateAuthorizedSession()

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

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

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

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

174+ assert.NoError(t, err)

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

176176}

177177

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

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

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

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

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

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

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

188188

189189 session := CreateAuthorizedSession()

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

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

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

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

191+ assert.NoError(t, err)

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

193193}

194194

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

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

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

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

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

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

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

205205

206206 session := CreateAuthorizedSession()

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

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

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

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

208+ assert.NoError(t, err)

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

210210}

211211

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

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

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

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

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

215215 })

216216 defer b.Close()

217217

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

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

221221

222222 session := CreateAuthorizedSession()

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

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

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

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

224+ assert.NoError(t, err)

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

226226}

227227

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

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

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

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

231231 })

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

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

237237

238238 session := CreateAuthorizedSession()

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

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

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

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

240+ assert.NoError(t, err)

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

242242}

243243

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

245245// 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) {

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

248248 defer b.Close()

249249

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

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

255255 // JSON to fail.

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

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

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

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

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

258+ assert.Error(t, err)

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

260260}

261261

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

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

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

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

265265 })

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

269269 p := testGitHubProvider(bURL.Host)

270270

271271 session := CreateAuthorizedSession()

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

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

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

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

273+ assert.Error(t, err)

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

275275}

276276

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

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

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

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

280280 })

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

284284 p := testGitHubProvider(bURL.Host)

285285

286286 session := CreateAuthorizedSession()

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

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

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

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

288+ assert.NoError(t, err)

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

290290}

291291

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

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

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

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

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

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

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

302302

303303 session := CreateAuthorizedSession()

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

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

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

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

305+ assert.NoError(t, err)

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

307307}

308308

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

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

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

311311 defer b.Close()

312312

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

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

316316

317317 session := CreateAuthorizedSession()

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

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

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

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

319+ assert.Error(t, err)

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

321321}

322322

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

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

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

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

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

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

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

333333

334334 session := CreateAuthorizedSession()

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

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

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

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

336+ assert.NoError(t, err)

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

338338}

339339

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

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

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

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

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

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

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

350350

351351 session := CreateAuthorizedSession()

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

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

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

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

353+ assert.Error(t, err)

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

355355}

356356

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

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

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

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

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

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

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

372372

373373 session := CreateAuthorizedSession()

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

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

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

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

375+ assert.NoError(t, err)

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

377377}

378378

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

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

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

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

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

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

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

384384 })

385385 defer b.Close()

386386

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

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

391391

392392 session := CreateAuthorizedSession()

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

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

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

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

394+ assert.NoError(t, err)

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

396396}