From fb1374f2f7c8cbb59dbef199e10be06df11596ef Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Tue, 1 Oct 2019 15:07:20 -0700 Subject: [PATCH] Rename iam/validator -> iam/openid and add tests (#8340) Refactor as part of config migration --- cmd/config-current.go | 14 +++--- cmd/config-migrate.go | 4 +- cmd/config-versions.go | 8 ++-- cmd/globals.go | 4 +- cmd/signature-v4-utils_test.go | 3 +- cmd/sts-handlers.go | 8 ++-- pkg/iam/{validator => openid}/jwks.go | 2 +- pkg/iam/{validator => openid}/jwks_test.go | 2 +- pkg/iam/{validator => openid}/jwt.go | 2 +- pkg/iam/{validator => openid}/jwt_test.go | 2 +- pkg/iam/{validator => openid}/validators.go | 2 +- .../{validator => openid}/validators_test.go | 46 +++++++++++++++++-- 12 files changed, 69 insertions(+), 28 deletions(-) rename pkg/iam/{validator => openid}/jwks.go (99%) rename pkg/iam/{validator => openid}/jwks_test.go (99%) rename pkg/iam/{validator => openid}/jwt.go (99%) rename pkg/iam/{validator => openid}/jwt_test.go (99%) rename pkg/iam/{validator => openid}/validators.go (99%) rename pkg/iam/{validator => openid}/validators_test.go (50%) diff --git a/cmd/config-current.go b/cmd/config-current.go index da86a59b6..c43990787 100644 --- a/cmd/config-current.go +++ b/cmd/config-current.go @@ -30,8 +30,8 @@ import ( "github.com/minio/minio/pkg/auth" "github.com/minio/minio/pkg/event" "github.com/minio/minio/pkg/event/target" + "github.com/minio/minio/pkg/iam/openid" iampolicy "github.com/minio/minio/pkg/iam/policy" - "github.com/minio/minio/pkg/iam/validator" xnet "github.com/minio/minio/pkg/net" ) @@ -571,7 +571,7 @@ func (s *serverConfig) loadToCachedConfigs() { "Unable to populate public key from JWKS URL %s", s.OpenID.JWKS.URL) } - globalIAMValidators = getAuthValidators(s) + globalIAMValidators = getOpenIDValidators(s) if s.Policy.OPA.URL != nil && s.Policy.OPA.URL.String() != "" { opaArgs := iampolicy.OpaArgs{ @@ -638,15 +638,15 @@ func loadConfig(objAPI ObjectLayer) error { return nil } -// getAuthValidators - returns ValidatorList which contains +// getOpenIDValidators - returns ValidatorList which contains // enabled providers in server config. // A new authentication provider is added like below -// * Add a new provider in pkg/iam/validator package. -func getAuthValidators(config *serverConfig) *validator.Validators { - validators := validator.NewValidators() +// * Add a new provider in pkg/iam/openid package. +func getOpenIDValidators(config *serverConfig) *openid.Validators { + validators := openid.NewValidators() if config.OpenID.JWKS.URL != nil { - validators.Add(validator.NewJWT(config.OpenID.JWKS)) + validators.Add(openid.NewJWT(config.OpenID.JWKS)) } return validators diff --git a/cmd/config-migrate.go b/cmd/config-migrate.go index 8a4159763..7defb8187 100644 --- a/cmd/config-migrate.go +++ b/cmd/config-migrate.go @@ -29,8 +29,8 @@ import ( "github.com/minio/minio/pkg/auth" "github.com/minio/minio/pkg/event" "github.com/minio/minio/pkg/event/target" + "github.com/minio/minio/pkg/iam/openid" iampolicy "github.com/minio/minio/pkg/iam/policy" - "github.com/minio/minio/pkg/iam/validator" xnet "github.com/minio/minio/pkg/net" "github.com/minio/minio/pkg/quick" ) @@ -2654,7 +2654,7 @@ func migrateV30ToV31MinioSys(objAPI ObjectLayer) error { } cfg.Version = "31" - cfg.OpenID.JWKS = validator.JWKSArgs{ + cfg.OpenID.JWKS = openid.JWKSArgs{ URL: &xnet.URL{}, } cfg.Policy.OPA = iampolicy.OpaArgs{ diff --git a/cmd/config-versions.go b/cmd/config-versions.go index bff7a3331..24ffec3c4 100644 --- a/cmd/config-versions.go +++ b/cmd/config-versions.go @@ -22,8 +22,8 @@ import ( "github.com/minio/minio/cmd/crypto" "github.com/minio/minio/pkg/auth" "github.com/minio/minio/pkg/event/target" + "github.com/minio/minio/pkg/iam/openid" iampolicy "github.com/minio/minio/pkg/iam/policy" - "github.com/minio/minio/pkg/iam/validator" "github.com/minio/minio/pkg/quick" ) @@ -817,7 +817,7 @@ type serverConfigV31 struct { // OpenID configuration OpenID struct { // JWKS validator config. - JWKS validator.JWKSArgs `json:"jwks"` + JWKS openid.JWKSArgs `json:"jwks"` } `json:"openid"` // External policy enforcements. @@ -872,7 +872,7 @@ type serverConfigV32 struct { // OpenID configuration OpenID struct { // JWKS validator config. - JWKS validator.JWKSArgs `json:"jwks"` + JWKS openid.JWKSArgs `json:"jwks"` } `json:"openid"` // External policy enforcements. @@ -916,7 +916,7 @@ type serverConfigV33 struct { // OpenID configuration OpenID struct { // JWKS validator config. - JWKS validator.JWKSArgs `json:"jwks"` + JWKS openid.JWKSArgs `json:"jwks"` } `json:"openid"` // External policy enforcements. diff --git a/cmd/globals.go b/cmd/globals.go index d5f0711b6..d5efb7de7 100644 --- a/cmd/globals.go +++ b/cmd/globals.go @@ -33,8 +33,8 @@ import ( "github.com/minio/minio/pkg/auth" "github.com/minio/minio/pkg/certs" "github.com/minio/minio/pkg/dns" + "github.com/minio/minio/pkg/iam/openid" iampolicy "github.com/minio/minio/pkg/iam/policy" - "github.com/minio/minio/pkg/iam/validator" "github.com/minio/minio/pkg/pubsub" ) @@ -269,7 +269,7 @@ var ( standardExcludeCompressContentTypes = []string{"video/*", "audio/*", "application/zip", "application/x-gzip", "application/x-zip-compressed", " application/x-compress", "application/x-spoon"} // Authorization validators list. - globalIAMValidators *validator.Validators + globalIAMValidators *openid.Validators // OPA policy system. globalPolicyOPA *iampolicy.Opa diff --git a/cmd/signature-v4-utils_test.go b/cmd/signature-v4-utils_test.go index c2dd61317..399cf50c1 100644 --- a/cmd/signature-v4-utils_test.go +++ b/cmd/signature-v4-utils_test.go @@ -17,9 +17,10 @@ package cmd import ( - "github.com/minio/minio/cmd/crypto" "net/http" "testing" + + "github.com/minio/minio/cmd/crypto" ) // TestSkipContentSha256Cksum - Test validate the logic which decides whether diff --git a/cmd/sts-handlers.go b/cmd/sts-handlers.go index 57d2db9c0..bc5008430 100644 --- a/cmd/sts-handlers.go +++ b/cmd/sts-handlers.go @@ -27,8 +27,8 @@ import ( xhttp "github.com/minio/minio/cmd/http" "github.com/minio/minio/cmd/logger" "github.com/minio/minio/pkg/auth" + "github.com/minio/minio/pkg/iam/openid" iampolicy "github.com/minio/minio/pkg/iam/policy" - "github.com/minio/minio/pkg/iam/validator" "github.com/minio/minio/pkg/wildcard" ldap "gopkg.in/ldap.v3" ) @@ -181,7 +181,7 @@ func (sts *stsAPIHandlers) AssumeRole(w http.ResponseWriter, r *http.Request) { var err error m := make(map[string]interface{}) - m["exp"], err = validator.GetDefaultExpiration(r.Form.Get("DurationSeconds")) + m["exp"], err = openid.GetDefaultExpiration(r.Form.Get("DurationSeconds")) if err != nil { writeSTSErrorResponse(ctx, w, ErrSTSInvalidParameterValue, err) return @@ -282,7 +282,7 @@ func (sts *stsAPIHandlers) AssumeRoleWithJWT(w http.ResponseWriter, r *http.Requ m, err := v.Validate(token, r.Form.Get("DurationSeconds")) if err != nil { switch err { - case validator.ErrTokenExpired: + case openid.ErrTokenExpired: switch action { case clientGrants: writeSTSErrorResponse(ctx, w, ErrSTSClientGrantsExpiredToken, err) @@ -290,7 +290,7 @@ func (sts *stsAPIHandlers) AssumeRoleWithJWT(w http.ResponseWriter, r *http.Requ writeSTSErrorResponse(ctx, w, ErrSTSWebIdentityExpiredToken, err) } return - case validator.ErrInvalidDuration: + case openid.ErrInvalidDuration: writeSTSErrorResponse(ctx, w, ErrSTSInvalidParameterValue, err) return } diff --git a/pkg/iam/validator/jwks.go b/pkg/iam/openid/jwks.go similarity index 99% rename from pkg/iam/validator/jwks.go rename to pkg/iam/openid/jwks.go index 728c6038a..0325543dd 100644 --- a/pkg/iam/validator/jwks.go +++ b/pkg/iam/openid/jwks.go @@ -14,7 +14,7 @@ * limitations under the License. */ -package validator +package openid import ( "crypto" diff --git a/pkg/iam/validator/jwks_test.go b/pkg/iam/openid/jwks_test.go similarity index 99% rename from pkg/iam/validator/jwks_test.go rename to pkg/iam/openid/jwks_test.go index 1b2bdef00..b4032f4a6 100644 --- a/pkg/iam/validator/jwks_test.go +++ b/pkg/iam/openid/jwks_test.go @@ -14,7 +14,7 @@ * limitations under the License. */ -package validator +package openid import ( "bytes" diff --git a/pkg/iam/validator/jwt.go b/pkg/iam/openid/jwt.go similarity index 99% rename from pkg/iam/validator/jwt.go rename to pkg/iam/openid/jwt.go index 6afed4e5f..b3febc1d0 100644 --- a/pkg/iam/validator/jwt.go +++ b/pkg/iam/openid/jwt.go @@ -14,7 +14,7 @@ * limitations under the License. */ -package validator +package openid import ( "crypto" diff --git a/pkg/iam/validator/jwt_test.go b/pkg/iam/openid/jwt_test.go similarity index 99% rename from pkg/iam/validator/jwt_test.go rename to pkg/iam/openid/jwt_test.go index 6f8535bdd..1075ff7ba 100644 --- a/pkg/iam/validator/jwt_test.go +++ b/pkg/iam/openid/jwt_test.go @@ -14,7 +14,7 @@ * limitations under the License. */ -package validator +package openid import ( "crypto" diff --git a/pkg/iam/validator/validators.go b/pkg/iam/openid/validators.go similarity index 99% rename from pkg/iam/validator/validators.go rename to pkg/iam/openid/validators.go index 2b213d332..4e2174402 100644 --- a/pkg/iam/validator/validators.go +++ b/pkg/iam/openid/validators.go @@ -14,7 +14,7 @@ * limitations under the License. */ -package validator +package openid import ( "errors" diff --git a/pkg/iam/validator/validators_test.go b/pkg/iam/openid/validators_test.go similarity index 50% rename from pkg/iam/validator/validators_test.go rename to pkg/iam/openid/validators_test.go index a222b74bb..7ed1bc5d4 100644 --- a/pkg/iam/validator/validators_test.go +++ b/pkg/iam/openid/validators_test.go @@ -14,10 +14,14 @@ * limitations under the License. */ -package validator +package openid import ( + "net/http" + "net/http/httptest" "testing" + + xnet "github.com/minio/minio/pkg/net" ) type errorValidator struct{} @@ -31,7 +35,32 @@ func (e errorValidator) ID() ID { } func TestValidators(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("content-type", "application/json") + w.Write([]byte(`{ + "keys" : [ { + "kty" : "RSA", + "kid" : "1438289820780", + "use" : "sig", + "alg" : "RS256", + "n" : "idWPro_QiAFOdMsJD163lcDIPogOwXogRo3Pct2MMyeE2GAGqV20Sc8QUbuLDfPl-7Hi9IfFOz--JY6QL5l92eV-GJXkTmidUEooZxIZSp3ghRxLCqlyHeF5LuuM5LPRFDeF4YWFQT_D2eNo_w95g6qYSeOwOwGIfaHa2RMPcQAiM6LX4ot-Z7Po9z0_3ztFa02m3xejEFr2rLRqhFl3FZJaNnwTUk6an6XYsunxMk3Ya3lRaKJReeXeFtfTpShgtPiAl7lIfLJH9h26h2OAlww531DpxHSm1gKXn6bjB0NTC55vJKft4wXoc_0xKZhnWmjQE8d9xE8e1Z3Ll1LYbw", + "e" : "AQAB" + }, { + "kty" : "RSA", + "kid" : "1438289856256", + "use" : "sig", + "alg" : "RS256", + "n" : "zo5cKcbFECeiH8eGx2D-DsFSpjSKbTVlXD6uL5JAy9rYIv7eYEP6vrKeX-x1z70yEdvgk9xbf9alc8siDfAz3rLCknqlqL7XGVAQL0ZP63UceDmD60LHOzMrx4eR6p49B3rxFfjvX2SWSV3-1H6XNyLk_ALbG6bGCFGuWBQzPJB4LMKCrOFq-6jtRKOKWBXYgkYkaYs5dG-3e2ULbq-y2RdgxYh464y_-MuxDQfvUgP787XKfcXP_XjJZvyuOEANjVyJYZSOyhHUlSGJapQ8ztHdF-swsnf7YkePJ2eR9fynWV2ZoMaXOdidgZtGTa4R1Z4BgH2C0hKJiqRy9fB7Gw", + "e" : "AQAB" + } ] +} +`)) + w.(http.Flusher).Flush() + })) + defer ts.Close() + vrs := NewValidators() + if err := vrs.Add(&errorValidator{}); err != nil { t.Fatal(err) } @@ -58,7 +87,18 @@ func TestValidators(t *testing.T) { t.Fatalf("Unexpected number of vids %v", vids) } - if vids[0] != "err" { - t.Fatalf("Unexpected vid %v", vids[0]) + u, err := xnet.ParseURL(ts.URL) + if err != nil { + t.Fatal(err) + } + + if err = vrs.Add(NewJWT(JWKSArgs{ + URL: u, + })); err != nil { + t.Fatal(err) + } + + if _, err = vrs.Get("jwt"); err != nil { + t.Fatal(err) } }