ea5b8cc · typeable/oauth2-proxy

git.iko.soy/typeable/oauth2-proxy

Support non-list and complex groups

commitea5b8cc21fb6c8feac6226b1131c45e9a173df2f authorNick Meves <nick.meves@greenhouse.io> dateSun 29 Nov 2020, 14:58 (6 years ago) parenteb56f24

6 files changed+166−36

compare with master

unifiedsplit

CHANGELOG.md+1 providers/oidc.go+9−9 providers/oidc_test.go+95−12 providers/provider_data.go+25−11 providers/provider_data_test.go+16−4 providers/util.go+20

CHANGELOG.md+1

@@ -48,6 +48,7 @@

4848- [#630](https://github.com/oauth2-proxy/oauth2-proxy/pull/630) Add support for Gitlab project based authentication (@factorysh)

4949- [#907](https://github.com/oauth2-proxy/oauth2-proxy/pull/907) Introduce alpha configuration option to enable testing of structured configuration (@JoelSpeed)

5050- [#938](https://github.com/oauth2-proxy/oauth2-proxy/pull/938) Cleanup missed provider renaming refactor methods (@NickMeves)

51+- [#816](https://github.com/oauth2-proxy/oauth2-proxy/pull/816) (via [#936](https://github.com/oauth2-proxy/oauth2-proxy/pull/936)) Support non-list group claims (@loafoe)

5152- [#936](https://github.com/oauth2-proxy/oauth2-proxy/pull/936) Refactor OIDC Provider and support groups from Profile URL (@NickMeves)

5253- [#925](https://github.com/oauth2-proxy/oauth2-proxy/pull/925) Fix basic auth legacy header conversion (@JoelSpeed)

5354- [#916](https://github.com/oauth2-proxy/oauth2-proxy/pull/916) Add AlphaOptions struct to prepare for alpha config loading (@JoelSpeed)

providers/oidc.go+9−9

@@ -4,6 +4,7 @@ import (

44 "context"

55 "errors"

66 "fmt"

7+ "reflect"

78 "time"

910 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions"

@@ -59,7 +60,7 @@ func (p *OIDCProvider) EnrichSession(ctx context.Context, s *sessions.SessionSta

5960 }

6061

6162 // Try to get missing emails or groups from a profileURL

62− if s.Email == "" || len(s.Groups) == 0 {

63+ if s.Email == "" || s.Groups == nil {

6364 err := p.callProfileURL(ctx, s)

6465 if err != nil {

6566 logger.Errorf("Warning: Profile URL request failed: %v", err)

@@ -90,16 +91,15 @@ func (p *OIDCProvider) callProfileURL(ctx context.Context, s *sessions.SessionSt

9091 s.Email = email

9192 }

9293

93− // Handle array & singleton groups cases

9494 if len(s.Groups) == 0 {

95− groups, err := respJSON.Get(p.GroupsClaim).StringArray()

96− if err == nil {

97− s.Groups = groups

98− } else {

99− group, err := respJSON.Get(p.GroupsClaim).String()

100− if err == nil {

101− s.Groups = []string{group}

95+ for _, group := range coerceArray(respJSON, p.GroupsClaim) {

96+ formatted, err := formatGroup(group)

97+ if err != nil {

98+ logger.Errorf("Warning: unable to format group of type %s with error %s",

99+ reflect.TypeOf(group), err)

100+ continue

102101 }

102+ s.Groups = append(s.Groups, formatted)

103103 }

104104 }

105105

providers/oidc_test.go+95−12

@@ -68,7 +68,7 @@ func newOIDCServer(body []byte) (*url.URL, *httptest.Server) {

6868 return u, s

6969}

7070

71−func newTestSetup(body []byte) (*httptest.Server, *OIDCProvider) {

71+func newTestOIDCSetup(body []byte) (*httptest.Server, *OIDCProvider) {

7272 redeemURL, server := newOIDCServer(body)

7373 provider := newOIDCProvider(redeemURL)

7474 return server, provider

@@ -85,7 +85,7 @@ func TestOIDCProviderRedeem(t *testing.T) {

8585 IDToken: idToken,

8686 })

8787

88− server, provider := newTestSetup(body)

88+ server, provider := newTestOIDCSetup(body)

8989 defer server.Close()

9090

9191 session, err := provider.Redeem(context.Background(), provider.RedeemURL.String(), "code1234")

@@ -108,7 +108,7 @@ func TestOIDCProviderRedeem_custom_userid(t *testing.T) {

108108 IDToken: idToken,

109109 })

110110

111− server, provider := newTestSetup(body)

111+ server, provider := newTestOIDCSetup(body)

112112 provider.EmailClaim = "phone_number"

113113 defer server.Close()

114114

@@ -247,7 +247,7 @@ func TestOIDCProvider_EnrichSession(t *testing.T) {

247247 ExistingSession: &sessions.SessionState{

248248 User: "already",

249249 Email: "already@populated.com",

250− Groups: []string{},

250+ Groups: nil,

251251 IDToken: idToken,

252252 AccessToken: accessToken,

253253 RefreshToken: refreshToken,

@@ -268,6 +268,89 @@ func TestOIDCProvider_EnrichSession(t *testing.T) {

268268 RefreshToken: refreshToken,

269269 },

270270 },

271+ "Missing Groups with Complex Groups in Profile URL": {

272+ ExistingSession: &sessions.SessionState{

273+ User: "already",

274+ Email: "already@populated.com",

275+ Groups: nil,

276+ IDToken: idToken,

277+ AccessToken: accessToken,

278+ RefreshToken: refreshToken,

279+ },

280+ EmailClaim: "email",

281+ GroupsClaim: "groups",

282+ ProfileJSON: map[string]interface{}{

283+ "email": "new@thing.com",

284+ "groups": []map[string]interface{}{

285+ {

286+ "groupId": "Admin Group Id",

287+ "roles": []string{"Admin"},

288+ },

289+ },

290+ },

291+ ExpectedError: nil,

292+ ExpectedSession: &sessions.SessionState{

293+ User: "already",

294+ Email: "already@populated.com",

295+ Groups: []string{"{\"groupId\":\"Admin Group Id\",\"roles\":[\"Admin\"]}"},

296+ IDToken: idToken,

297+ AccessToken: accessToken,

298+ RefreshToken: refreshToken,

299+ },

300+ },

301+ "Missing Groups with Singleton Complex Group in Profile URL": {

302+ ExistingSession: &sessions.SessionState{

303+ User: "already",

304+ Email: "already@populated.com",

305+ Groups: nil,

306+ IDToken: idToken,

307+ AccessToken: accessToken,

308+ RefreshToken: refreshToken,

309+ },

310+ EmailClaim: "email",

311+ GroupsClaim: "groups",

312+ ProfileJSON: map[string]interface{}{

313+ "email": "new@thing.com",

314+ "groups": map[string]interface{}{

315+ "groupId": "Admin Group Id",

316+ "roles": []string{"Admin"},

317+ },

318+ },

319+ ExpectedError: nil,

320+ ExpectedSession: &sessions.SessionState{

321+ User: "already",

322+ Email: "already@populated.com",

323+ Groups: []string{"{\"groupId\":\"Admin Group Id\",\"roles\":[\"Admin\"]}"},

324+ IDToken: idToken,

325+ AccessToken: accessToken,

326+ RefreshToken: refreshToken,

327+ },

328+ },

329+ "Empty Groups Claims": {

330+ ExistingSession: &sessions.SessionState{

331+ User: "already",

332+ Email: "already@populated.com",

333+ Groups: []string{},

334+ IDToken: idToken,

335+ AccessToken: accessToken,

336+ RefreshToken: refreshToken,

337+ },

338+ EmailClaim: "email",

339+ GroupsClaim: "groups",

340+ ProfileJSON: map[string]interface{}{

341+ "email": "new@thing.com",

342+ "groups": []string{"new", "thing"},

343+ },

344+ ExpectedError: nil,

345+ ExpectedSession: &sessions.SessionState{

346+ User: "already",

347+ Email: "already@populated.com",

348+ Groups: []string{},

349+ IDToken: idToken,

350+ AccessToken: accessToken,

351+ RefreshToken: refreshToken,

352+ },

353+ },

271354 "Missing Groups with Custom Claim": {

272355 ExistingSession: &sessions.SessionState{

273356 User: "already",

@@ -297,7 +380,7 @@ func TestOIDCProvider_EnrichSession(t *testing.T) {

297380 ExistingSession: &sessions.SessionState{

298381 User: "already",

299382 Email: "already@populated.com",

300− Groups: []string{},

383+ Groups: nil,

301384 IDToken: idToken,

302385 AccessToken: accessToken,

303386 RefreshToken: refreshToken,

@@ -346,7 +429,7 @@ func TestOIDCProvider_EnrichSession(t *testing.T) {

346429 jsonResp, err := json.Marshal(tc.ProfileJSON)

347430 assert.NoError(t, err)

348431

349− server, provider := newTestSetup(jsonResp)

432+ server, provider := newTestOIDCSetup(jsonResp)

350433 provider.ProfileURL, err = url.Parse(server.URL)

351434 assert.NoError(t, err)

352435

@@ -371,7 +454,7 @@ func TestOIDCProviderRefreshSessionIfNeededWithoutIdToken(t *testing.T) {

371454 RefreshToken: refreshToken,

372455 })

373456

374− server, provider := newTestSetup(body)

457+ server, provider := newTestOIDCSetup(body)

375458 defer server.Close()

376459

377460 existingSession := &sessions.SessionState{

@@ -405,7 +488,7 @@ func TestOIDCProviderRefreshSessionIfNeededWithIdToken(t *testing.T) {

405488 IDToken: idToken,

406489 })

407490

408− server, provider := newTestSetup(body)

491+ server, provider := newTestOIDCSetup(body)

409492 defer server.Close()

410493

411494 existingSession := &sessions.SessionState{

@@ -433,7 +516,7 @@ func TestOIDCProviderCreateSessionFromToken(t *testing.T) {

433516 GroupsClaim string

434517 ExpectedUser string

435518 ExpectedEmail string

436− ExpectedGroups interface{}

519+ ExpectedGroups []string

437520 }{

438521 "Default IDToken": {

439522 IDToken: defaultIDToken,

@@ -447,7 +530,7 @@ func TestOIDCProviderCreateSessionFromToken(t *testing.T) {

447530 GroupsClaim: "groups",

448531 ExpectedUser: "123456789",

449532 ExpectedEmail: "123456789",

450− ExpectedGroups: []string{},

533+ ExpectedGroups: nil,

451534 },

452535 "Custom Groups Claim": {

453536 IDToken: defaultIDToken,

@@ -466,7 +549,7 @@ func TestOIDCProviderCreateSessionFromToken(t *testing.T) {

466549 }

467550 for testName, tc := range testCases {

468551 t.Run(testName, func(t *testing.T) {

469− server, provider := newTestSetup([]byte(`{}`))

552+ server, provider := newTestOIDCSetup([]byte(`{}`))

470553 provider.GroupsClaim = tc.GroupsClaim

471554 defer server.Close()

472555

@@ -478,9 +561,9 @@ func TestOIDCProviderCreateSessionFromToken(t *testing.T) {

478561

479562 assert.Equal(t, tc.ExpectedUser, ss.User)

480563 assert.Equal(t, tc.ExpectedEmail, ss.Email)

564+ assert.Equal(t, tc.ExpectedGroups, ss.Groups)

481565 assert.Equal(t, rawIDToken, ss.IDToken)

482566 assert.Equal(t, rawIDToken, ss.AccessToken)

483− assert.Equal(t, tc.ExpectedGroups, ss.Groups)

484567 assert.Equal(t, "", ss.RefreshToken)

485568 })

486569 }

providers/provider_data.go+25−11

@@ -189,20 +189,34 @@ func (p *ProviderData) getClaims(idToken *oidc.IDToken) (*OIDCClaims, error) {

189189 return claims, nil

190190}

191191

192−// extractGroups extracts groups from a claim to a list in a type safe manner

192+// extractGroups extracts groups from a claim to a list in a type safe manner.

193+// If the claim isn't present, `nil` is returned. If the groups claim is

194+// present but empty, `[]string{}` is returned.

193195func (p *ProviderData) extractGroups(claims map[string]interface{}) []string {

196+ rawClaim, ok := claims[p.GroupsClaim]

197+ if !ok {

198+ return nil

199+ }

200+

201+ // Handle traditional list-based groups as well as non-standard singleton

202+ // based groups. Both variants support complex objects if needed.

203+ var claimGroups []interface{}

204+ switch raw := rawClaim.(type) {

205+ case []interface{}:

206+ claimGroups = raw

207+ case interface{}:

208+ claimGroups = []interface{}{raw}

209+ }

210+

194211 groups := []string{}

195− rawGroups, ok := claims[p.GroupsClaim].([]interface{})

196− if rawGroups != nil && ok {

197− for _, rawGroup := range rawGroups {

198− formattedGroup, err := formatGroup(rawGroup)

199− if err != nil {

200− logger.Errorf("Warning: unable to format group of type %s with error %s",

201− reflect.TypeOf(rawGroup), err)

202− continue

203− }

204− groups = append(groups, formattedGroup)

212+ for _, rawGroup := range claimGroups {

213+ formattedGroup, err := formatGroup(rawGroup)

214+ if err != nil {

215+ logger.Errorf("Warning: unable to format group of type %s with error %s",

216+ reflect.TypeOf(rawGroup), err)

217+ continue

205218 }

219+ groups = append(groups, formattedGroup)

206220 }

207221 return groups

208222}

providers/provider_data_test.go+16−4

@@ -300,7 +300,7 @@ func TestProviderData_buildSessionFromClaims(t *testing.T) {

300300 ExpectedSession: &sessions.SessionState{

301301 User: "123456789",

302302 Email: "janed@me.com",

303− Groups: []string{},

303+ Groups: nil,

304304 PreferredUsername: "Jane Dobbs",

305305 },

306306 },

@@ -386,12 +386,20 @@ func TestProviderData_extractGroups(t *testing.T) {

386386 "Just::A::String",

387387 },

388388 },

389− "Missing Groups": {

389+ "Missing Groups Claim Returns Nil": {

390390 Claims: map[string]interface{}{

391391 "email": "this@does.not.matter.com",

392392 },

393393 GroupsClaim: "groups",

394− ExpectedGroups: []string{},

394+ ExpectedGroups: nil,

395+ },

396+ "Non List Groups": {

397+ Claims: map[string]interface{}{

398+ "email": "this@does.not.matter.com",

399+ "groups": "singleton",

400+ },

401+ GroupsClaim: "groups",

402+ ExpectedGroups: []string{"singleton"},

395403 },

396404 }

397405 for testName, tc := range testCases {

@@ -408,7 +416,11 @@ func TestProviderData_extractGroups(t *testing.T) {

408416 provider.GroupsClaim = tc.GroupsClaim

409417

410418 groups := provider.extractGroups(tc.Claims)

411− g.Expect(groups).To(Equal(tc.ExpectedGroups))

419+ if tc.ExpectedGroups != nil {

420+ g.Expect(groups).To(Equal(tc.ExpectedGroups))

421+ } else {

422+ g.Expect(groups).To(BeNil())

423+ }

412424 })

413425 }

414426}

providers/util.go+20

@@ -6,6 +6,7 @@ import (

66 "net/http"

77 "net/url"

9+ "github.com/bitly/go-simplejson"

910 "golang.org/x/oauth2"

1011)

1112

@@ -59,6 +60,8 @@ func makeLoginURL(p *ProviderData, redirectURI, state string, extraParams url.Va

5960 return a

6061}

6162

63+// getIDToken extracts an IDToken stored in the `Extra` fields of an

64+// oauth2.Token

6265func getIDToken(token *oauth2.Token) string {

6366 idToken, ok := token.Extra("id_token").(string)

6467 if !ok {

@@ -67,6 +70,8 @@ func getIDToken(token *oauth2.Token) string {

6770 return idToken

6871}

6972

73+// formatGroup coerces an OIDC groups claim into a string

74+// If it is non-string, marshal it into JSON.

7075func formatGroup(rawGroup interface{}) (string, error) {

7176 group, ok := rawGroup.(string)

7277 if !ok {

@@ -78,3 +83,18 @@ func formatGroup(rawGroup interface{}) (string, error) {

7883 }

7984 return group, nil

8085}

86+

87+// coerceArray extracts a field from simplejson.Json that might be a

88+// singleton or a list and coerces it into a list.

89+func coerceArray(sj *simplejson.Json, key string) []interface{} {

90+ array, err := sj.Get(key).Array()

91+ if err == nil {

92+ return array

93+ }

94+

95+ single := sj.Get(key).Interface()

96+ if single == nil {

97+ return nil

98+ }

99+ return []interface{}{single}

100+}