fix: enforce deny if present for implicit permissions (#11680)

Implicit permissions for any user is to be allowed to
change their own password, we need to restrict this
further even if there is an implicit allow for this
scenario - we have to honor Deny statements if they
are specified.

browser/app/js/browser/__tests__/ChangePasswordModal.test.js

@@ -24,9 +24,6 @@ jest.mock("jwt-decode")
 jwtDecode.mockImplementation(() => ({ sub: "minio" }))
 
 jest.mock("../../web", () => ({
-  GenerateAuth: jest.fn(() => {
-    return Promise.resolve({ accessKey: "gen1", secretKey: "gen2" })
-  }),
   SetAuth: jest.fn(
     ({ currentAccessKey, currentSecretKey, newAccessKey, newSecretKey }) => {
       if (

cmd/admin-handlers-users.go

@@ -399,7 +399,7 @@ func (a adminAPIHandlers) AddUser(w http.ResponseWriter, r *http.Request) {
 			AccountName:     parentUser,
 			Action:          iampolicy.CreateUserAdminAction,
 			ConditionValues: getConditionValues(r, "", parentUser, claims),
-			IsOwner:         owner,
+			IsOwner:         false,
 			Claims:          claims,
 		}) {
 			writeErrorResponseJSON(ctx, w, errorCodes.ToAPIErr(ErrAccessDenied), r.URL)
@@ -407,6 +407,18 @@ func (a adminAPIHandlers) AddUser(w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
+	if implicitPerm && !globalIAMSys.IsAllowed(iampolicy.Args{
+		AccountName:     accessKey,
+		Action:          iampolicy.CreateUserAdminAction,
+		ConditionValues: getConditionValues(r, "", accessKey, claims),
+		IsOwner:         false,
+		Claims:          claims,
+		DenyOnly:        true, // check if changing password is explicitly denied.
+	}) {
+		writeErrorResponseJSON(ctx, w, errorCodes.ToAPIErr(ErrAccessDenied), r.URL)
+		return
+	}
+
 	if r.ContentLength > maxEConfigJSONSize || r.ContentLength == -1 {
 		// More than maxConfigSize bytes were available
 		writeErrorResponseJSON(ctx, w, errorCodes.ToAPIErr(ErrAdminConfigTooLarge), r.URL)

cmd/logger/logger.go

@@ -72,9 +72,7 @@ var matchingFuncNames = [...]string{
 	"cmd.(*webAPIHandlers).ListObjects",
 	"cmd.(*webAPIHandlers).RemoveObject",
 	"cmd.(*webAPIHandlers).Login",
-	"cmd.(*webAPIHandlers).GenerateAuth",
 	"cmd.(*webAPIHandlers).SetAuth",
-	"cmd.(*webAPIHandlers).GetAuth",
 	"cmd.(*webAPIHandlers).CreateURLToken",
 	"cmd.(*webAPIHandlers).Upload",
 	"cmd.(*webAPIHandlers).Download",

cmd/web-handlers.go

@@ -78,7 +78,7 @@ func extractBucketObject(args reflect.Value) (bucketName, objectName string) {
 }
 
 // WebGenericArgs - empty struct for calls that don't accept arguments
-// for ex. ServerInfo, GenerateAuth
+// for ex. ServerInfo
 type WebGenericArgs struct{}
 
 // WebGenericRep - reply structure for calls for which reply is success/failure
@@ -981,32 +981,6 @@ func (web *webAPIHandlers) Login(r *http.Request, args *LoginArgs, reply *LoginR
 	return nil
 }
 
-// GenerateAuthReply - reply for GenerateAuth
-type GenerateAuthReply struct {
-	AccessKey string `json:"accessKey"`
-	SecretKey string `json:"secretKey"`
-	UIVersion string `json:"uiVersion"`
-}
-
-func (web webAPIHandlers) GenerateAuth(r *http.Request, args *WebGenericArgs, reply *GenerateAuthReply) error {
-	ctx := newWebContext(r, args, "WebGenerateAuth")
-	_, owner, authErr := webRequestAuthenticate(r)
-	if authErr != nil {
-		return toJSONError(ctx, authErr)
-	}
-	if !owner {
-		return toJSONError(ctx, errAccessDenied)
-	}
-	cred, err := auth.GetNewCredentials()
-	if err != nil {
-		return toJSONError(ctx, err)
-	}
-	reply.AccessKey = cred.AccessKey
-	reply.SecretKey = cred.SecretKey
-	reply.UIVersion = browser.UIVersion
-	return nil
-}
-
 // SetAuthArgs - argument for SetAuth
 type SetAuthArgs struct {
 	CurrentAccessKey string `json:"currentAccessKey"`
@@ -1035,6 +1009,17 @@ func (web *webAPIHandlers) SetAuth(r *http.Request, args *SetAuthArgs, reply *Se
 		return toJSONError(ctx, errChangeCredNotAllowed)
 	}
 
+	if !globalIAMSys.IsAllowed(iampolicy.Args{
+		AccountName:     claims.AccessKey,
+		Action:          iampolicy.CreateUserAdminAction,
+		IsOwner:         false,
+		ConditionValues: getConditionValues(r, "", claims.AccessKey, claims.Map()),
+		Claims:          claims.Map(),
+		DenyOnly:        true,
+	}) {
+		return toJSONError(ctx, errChangeCredNotAllowed)
+	}
+
 	// for IAM users, access key cannot be updated
 	// claims.AccessKey is used instead of accesskey from args
 	prevCred, ok := globalIAMSys.GetUser(claims.AccessKey)

cmd/web-handlers_test.go

@@ -631,43 +631,6 @@ func testRemoveObjectWebHandler(obj ObjectLayer, instanceType string, t TestErrH
 	}
 }
 
-// Wrapper for calling Generate Auth Handler
-func TestWebHandlerGenerateAuth(t *testing.T) {
-	ExecObjectLayerTest(t, testGenerateAuthWebHandler)
-}
-
-// testGenerateAuthWebHandler - Test GenerateAuth web handler
-func testGenerateAuthWebHandler(obj ObjectLayer, instanceType string, t TestErrHandler) {
-	// Register the API end points with Erasure/FS object layer.
-	apiRouter := initTestWebRPCEndPoint(obj)
-	credentials := globalActiveCred
-
-	rec := httptest.NewRecorder()
-	authorization, err := getWebRPCToken(apiRouter, credentials.AccessKey, credentials.SecretKey)
-	if err != nil {
-		t.Fatal("Cannot authenticate")
-	}
-
-	generateAuthRequest := WebGenericArgs{}
-	generateAuthReply := &GenerateAuthReply{}
-	req, err := newTestWebRPCRequest("web.GenerateAuth", authorization, generateAuthRequest)
-	if err != nil {
-		t.Fatalf("Failed to create HTTP request: <ERROR> %v", err)
-	}
-	apiRouter.ServeHTTP(rec, req)
-	if rec.Code != http.StatusOK {
-		t.Fatalf("Expected the response status to be 200, but instead found `%d`", rec.Code)
-	}
-	err = getTestWebRPCResponse(rec, &generateAuthReply)
-	if err != nil {
-		t.Fatalf("Failed, %v", err)
-	}
-
-	if generateAuthReply.AccessKey == "" || generateAuthReply.SecretKey == "" {
-		t.Fatalf("Failed to generate auth keys")
-	}
-}
-
 func TestWebCreateURLToken(t *testing.T) {
 	ExecObjectLayerTest(t, testCreateURLToken)
 }
@@ -1134,9 +1097,8 @@ func TestWebCheckAuthorization(t *testing.T) {
 	webRPCs := []string{
 		"ServerInfo", "StorageInfo", "MakeBucket",
 		"ListBuckets", "ListObjects", "RemoveObject",
-		"GenerateAuth", "SetAuth",
+		"SetAuth", "GetBucketPolicy", "SetBucketPolicy",
-		"GetBucketPolicy", "SetBucketPolicy", "ListAllBucketPolicies",
+		"ListAllBucketPolicies", "PresignedGet",
-		"PresignedGet",
 	}
 	for _, rpcCall := range webRPCs {
 		reply := &WebGenericRep{}

pkg/iam/policy/policy.go

@@ -37,6 +37,7 @@ type Args struct {
 	IsOwner         bool                   `json:"owner"`
 	ObjectName      string                 `json:"object"`
 	Claims          map[string]interface{} `json:"claims"`
+	DenyOnly        bool                   `json:"denyOnly"` // only applies deny
 }
 
 // GetPoliciesFromClaims returns the list of policies to be applied for this
@@ -105,6 +106,15 @@ func (iamp Policy) IsAllowed(args Args) bool {
 		}
 	}
 
+	// Applied any 'Deny' only policies, if we have
+	// reached here it means that there were no 'Deny'
+	// policies - this function mainly used for
+	// specific scenarios where we only want to validate
+	// 'Deny' only policies.
+	if args.DenyOnly {
+		return true
+	}
+
 	// For owner, its allowed by default.
 	if args.IsOwner {
 		return true