ea5b8cc · typeable/oauth2-proxy

git.iko.soy/typeable/oauth2-proxy

Support non-list and complex groups

ea5b8ccNick Meves6 years ago

CHANGELOG.md+1

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

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

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

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

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

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

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

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

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

54- [#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 (

4 "context"

5 "errors"

6 "fmt"

7+ "reflect"

7 "time"

8 "time"

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

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

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

59 }

60 }

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

62 // 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 {

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

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

64 if err != nil {

65 if err != nil {

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

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

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

90 s.Email = email

91 s.Email = email

91 }

92 }

93− // Handle array & singleton groups cases

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

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

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

96+ formatted, err := formatGroup(group)

96− if err == nil {

97+ if err != nil {

97− s.Groups = groups

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

98− } else {

99+ reflect.TypeOf(group), err)

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

100+ continue

100− if err == nil {

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

102 }

101 }

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

103 }

104 }

105

providers/oidc_test.go+95−12

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

68 return u, s

69}

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

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

72 redeemURL, server := newOIDCServer(body)

73 provider := newOIDCProvider(redeemURL)

74 return server, provider

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

85 IDToken: idToken,

86 })

88− server, provider := newTestSetup(body)

88+ server, provider := newTestOIDCSetup(body)

89 defer server.Close()

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

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

108 IDToken: idToken,

109 })

110

111− server, provider := newTestSetup(body)

111+ server, provider := newTestOIDCSetup(body)

112 provider.EmailClaim = "phone_number"

113 defer server.Close()

114

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

247 ExistingSession: &sessions.SessionState{

248 User: "already",

249 Email: "already@populated.com",

250− Groups: []string{},

250+ Groups: nil,

251 IDToken: idToken,

252 AccessToken: accessToken,

253 RefreshToken: refreshToken,

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

268 RefreshToken: refreshToken,

269 },

270 },

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+ },

271 "Missing Groups with Custom Claim": {

354 "Missing Groups with Custom Claim": {

272 ExistingSession: &sessions.SessionState{

355 ExistingSession: &sessions.SessionState{

273 User: "already",

356 User: "already",

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

297 ExistingSession: &sessions.SessionState{

380 ExistingSession: &sessions.SessionState{

298 User: "already",

381 User: "already",

299 Email: "already@populated.com",

382 Email: "already@populated.com",

300− Groups: []string{},

383+ Groups: nil,

301 IDToken: idToken,

384 IDToken: idToken,

302 AccessToken: accessToken,

385 AccessToken: accessToken,

303 RefreshToken: refreshToken,

386 RefreshToken: refreshToken,

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

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

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

347 assert.NoError(t, err)

430 assert.NoError(t, err)

348

431

349− server, provider := newTestSetup(jsonResp)

432+ server, provider := newTestOIDCSetup(jsonResp)

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

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

351 assert.NoError(t, err)

434 assert.NoError(t, err)

352

435

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

371 RefreshToken: refreshToken,

454 RefreshToken: refreshToken,

372 })

455 })

373

456

374− server, provider := newTestSetup(body)

457+ server, provider := newTestOIDCSetup(body)

375 defer server.Close()

458 defer server.Close()

376

459

377 existingSession := &sessions.SessionState{

460 existingSession := &sessions.SessionState{

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

405 IDToken: idToken,

488 IDToken: idToken,

406 })

489 })

407

490

408− server, provider := newTestSetup(body)

491+ server, provider := newTestOIDCSetup(body)

409 defer server.Close()

492 defer server.Close()

410

493

411 existingSession := &sessions.SessionState{

494 existingSession := &sessions.SessionState{

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

433 GroupsClaim string

516 GroupsClaim string

434 ExpectedUser string

517 ExpectedUser string

435 ExpectedEmail string

518 ExpectedEmail string

436− ExpectedGroups interface{}

519+ ExpectedGroups []string

437 }{

520 }{

438 "Default IDToken": {

521 "Default IDToken": {

439 IDToken: defaultIDToken,

522 IDToken: defaultIDToken,

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

447 GroupsClaim: "groups",

530 GroupsClaim: "groups",

448 ExpectedUser: "123456789",

531 ExpectedUser: "123456789",

449 ExpectedEmail: "123456789",

532 ExpectedEmail: "123456789",

450− ExpectedGroups: []string{},

533+ ExpectedGroups: nil,

451 },

534 },

452 "Custom Groups Claim": {

535 "Custom Groups Claim": {

453 IDToken: defaultIDToken,

536 IDToken: defaultIDToken,

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

466 }

549 }

467 for testName, tc := range testCases {

550 for testName, tc := range testCases {

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

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

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

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

470 provider.GroupsClaim = tc.GroupsClaim

553 provider.GroupsClaim = tc.GroupsClaim

471 defer server.Close()

554 defer server.Close()

472

555

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

478

561

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

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

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

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

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

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

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

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

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

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

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

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

485 })

568 })

486 }

569 }

providers/provider_data.go+25−11

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

189 return claims, nil

190}

191

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.

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

195func (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+

194 groups := []string{}

211 groups := []string{}

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

196− if rawGroups != nil && ok {

197− for _, rawGroup := range rawGroups {

212+ for _, rawGroup := range claimGroups {

198− formattedGroup, err := formatGroup(rawGroup)

213+ formattedGroup, err := formatGroup(rawGroup)

199− if err != nil {

214+ if err != nil {

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

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

201− reflect.TypeOf(rawGroup), err)

216+ reflect.TypeOf(rawGroup), err)

202− continue

217+ continue

203− }

204− groups = append(groups, formattedGroup)

205 }

218 }

219+ groups = append(groups, formattedGroup)

206 }

220 }

207 return groups

221 return groups

208}

222}

providers/provider_data_test.go+16−4

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

300 ExpectedSession: &sessions.SessionState{

301 User: "123456789",

302 Email: "janed@me.com",

303− Groups: []string{},

303+ Groups: nil,

304 PreferredUsername: "Jane Dobbs",

305 },

306 },

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

386 "Just::A::String",

387 },

388 },

389− "Missing Groups": {

389+ "Missing Groups Claim Returns Nil": {

390 Claims: map[string]interface{}{

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

392 },

393 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"},

395 },

403 },

396 }

404 }

397 for testName, tc := range testCases {

405 for testName, tc := range testCases {

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

408 provider.GroupsClaim = tc.GroupsClaim

416 provider.GroupsClaim = tc.GroupsClaim

409

417

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

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

419+ if tc.ExpectedGroups != nil {

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

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

421+ } else {

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

423+ }

412 })

424 })

413 }

425 }

414}

426}

providers/util.go+20

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

6 "net/http"

7 "net/url"

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

9 "golang.org/x/oauth2"

10 "golang.org/x/oauth2"

10)

11)

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

59 return a

60 return a

60}

61}

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

64+// oauth2.Token

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

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

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

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

64 if !ok {

67 if !ok {

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

67 return idToken

70 return idToken

68}

71}

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

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

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

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

71 group, ok := rawGroup.(string)

76 group, ok := rawGroup.(string)

72 if !ok {

77 if !ok {

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

78 }

83 }

79 return group, nil

84 return group, nil

80}

85}

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+}