fix: allow accountInfo, addUser and getUserInfo implicit (#10978)

- accountInfo API that returns information about user, access to buckets and the size per bucket - addUser - user is allowed to change their secretKey - getUserInfo - returns user info if the incoming is the same user requesting their information
2020-11-27 17:23:57 -08:00 · 2020-11-27 17:23:57 -08:00 · e6fa410778
parent 350c5ff8f8
commit e6fa410778
7 changed files with 142 additions and 64 deletions
--- a/cmd/admin-handlers-users.go
+++ b/cmd/admin-handlers-users.go
@ -129,13 +129,40 @@ func (a adminAPIHandlers) GetUserInfo(w http.ResponseWriter, r *http.Request) {

 	defer logger.AuditLog(w, r, "GetUserInfo", mustGetClaimsFromToken(r))

-	objectAPI, _ := validateAdminUsersReq(ctx, w, r, iampolicy.GetUserAdminAction)
-	if objectAPI == nil {
+	vars := mux.Vars(r)
+	name := vars["accessKey"]
+
+	// Get current object layer instance.
+	objectAPI := newObjectLayerFn()
+	if objectAPI == nil || globalNotificationSys == nil {
+		writeErrorResponseJSON(ctx, w, errorCodes.ToAPIErr(ErrServerNotInitialized), r.URL)
 		return
 	}

-	vars := mux.Vars(r)
-	name := vars["accessKey"]
+	cred, claims, owner, s3Err := validateAdminSignature(ctx, r, "")
+	if s3Err != ErrNone {
+		writeErrorResponseJSON(ctx, w, errorCodes.ToAPIErr(s3Err), r.URL)
+		return
+	}
+
+	accessKey := cred.AccessKey
+	if cred.ParentUser != "" {
+		accessKey = cred.ParentUser
+	}
+
+	implicitPerm := name == accessKey
+	if !implicitPerm {
+		if !globalIAMSys.IsAllowed(iampolicy.Args{
+			AccountName:     accessKey,
+			Action:          iampolicy.GetUserAdminAction,
+			ConditionValues: getConditionValues(r, "", accessKey, claims),
+			IsOwner:         owner,
+			Claims:          claims,
+		}) {
+			writeErrorResponseJSON(ctx, w, errorCodes.ToAPIErr(ErrAccessDenied), r.URL)
+			return
+		}
+	}

 	userInfo, err := globalIAMSys.GetUserInfo(name)
 	if err != nil {
@ -304,7 +331,7 @@ func (a adminAPIHandlers) SetUserStatus(w http.ResponseWriter, r *http.Request)
 	accessKey := vars["accessKey"]
 	status := vars["status"]

-	// Custom IAM policies not allowed for admin user.
+	// This API is not allowed to lookup accessKey user status
 	if accessKey == globalActiveCred.AccessKey {
 		writeErrorResponseJSON(ctx, w, errorCodes.ToAPIErr(ErrInvalidRequest), r.URL)
 		return
@ -330,20 +357,47 @@ func (a adminAPIHandlers) AddUser(w http.ResponseWriter, r *http.Request) {

 	defer logger.AuditLog(w, r, "AddUser", mustGetClaimsFromToken(r))

-	objectAPI, cred := validateAdminUsersReq(ctx, w, r, iampolicy.CreateUserAdminAction)
-	if objectAPI == nil {
-		return
-	}
-
 	vars := mux.Vars(r)
 	accessKey := vars["accessKey"]

-	// Custom IAM policies not allowed for admin user.
-	if accessKey == globalActiveCred.AccessKey {
+	// Get current object layer instance.
+	objectAPI := newObjectLayerFn()
+	if objectAPI == nil || globalNotificationSys == nil {
+		writeErrorResponseJSON(ctx, w, errorCodes.ToAPIErr(ErrServerNotInitialized), r.URL)
+		return
+	}
+
+	cred, claims, owner, s3Err := validateAdminSignature(ctx, r, "")
+	if s3Err != ErrNone {
+		writeErrorResponseJSON(ctx, w, errorCodes.ToAPIErr(s3Err), r.URL)
+		return
+	}
+
+	if cred.IsTemp() || cred.IsServiceAccount() {
+		writeErrorResponseJSON(ctx, w, errorCodes.ToAPIErr(ErrAccountNotEligible), r.URL)
+		return
+	}
+
+	// Not allowed to add a user with same access key as root credential
+	if owner {
 		writeErrorResponseJSON(ctx, w, errorCodes.ToAPIErr(ErrAddUserInvalidArgument), r.URL)
 		return
 	}

+	implicitPerm := accessKey == cred.AccessKey
+	if !implicitPerm {
+		if !globalIAMSys.IsAllowed(iampolicy.Args{
+			AccountName:     accessKey,
+			Action:          iampolicy.CreateUserAdminAction,
+			ConditionValues: getConditionValues(r, "", accessKey, claims),
+			IsOwner:         owner,
+			Claims:          claims,
+		}) {
+			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)
@ -398,6 +452,12 @@ func (a adminAPIHandlers) AddServiceAccount(w http.ResponseWriter, r *http.Reque
 		return
 	}

+	// Disallow creating service accounts by root user.
+	if owner {
+		writeErrorResponseJSON(ctx, w, errorCodes.ToAPIErr(ErrAdminAccountNotEligible), r.URL)
+		return
+	}
+
 	password := cred.SecretKey
 	reqBytes, err := madmin.DecryptData(password, io.LimitReader(r.Body, r.ContentLength))
 	if err != nil {
@ -411,12 +471,6 @@ func (a adminAPIHandlers) AddServiceAccount(w http.ResponseWriter, r *http.Reque
 		return
 	}

-	// Disallow creating service accounts by root user.
-	if owner {
-		writeErrorResponseJSON(ctx, w, errorCodes.ToAPIErr(ErrAdminAccountNotEligible), r.URL)
-		return
-	}
-
 	parentUser := cred.AccessKey
 	if cred.ParentUser != "" {
 		parentUser = cred.ParentUser
@ -572,11 +626,11 @@ func (a adminAPIHandlers) DeleteServiceAccount(w http.ResponseWriter, r *http.Re
 	writeSuccessNoContent(w)
 }

-// AccountUsageInfoHandler returns usage
-func (a adminAPIHandlers) AccountUsageInfoHandler(w http.ResponseWriter, r *http.Request) {
-	ctx := newContext(r, w, "AccountUsageInfo")
+// AccountInfoHandler returns usage
+func (a adminAPIHandlers) AccountInfoHandler(w http.ResponseWriter, r *http.Request) {
+	ctx := newContext(r, w, "AccountInfo")

-	defer logger.AuditLog(w, r, "AccountUsageInfo", mustGetClaimsFromToken(r))
+	defer logger.AuditLog(w, r, "AccountInfo", mustGetClaimsFromToken(r))

 	// Get current object layer instance.
 	objectAPI := newObjectLayerFn()
@ -645,8 +699,16 @@ func (a adminAPIHandlers) AccountUsageInfoHandler(w http.ResponseWriter, r *http
 		accountName = cred.ParentUser
 	}

-	acctInfo := madmin.AccountUsageInfo{
+	policies, err := globalIAMSys.PolicyDBGet(accountName, false)
+	if err != nil {
+		logger.LogIf(ctx, err)
+		writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL)
+		return
+	}
+
+	acctInfo := madmin.AccountInfo{
 		AccountName: accountName,
+		Policy:      globalIAMSys.GetCombinedPolicy(policies...),
 	}

 	for _, bucket := range buckets {
--- a/cmd/admin-router.go
+++ b/cmd/admin-router.go
@ -116,7 +116,7 @@ func registerAdminRouter(router *mux.Router, enableConfigOps, enableIAMOps bool)

 			// Add user IAM

-			adminRouter.Methods(http.MethodGet).Path(adminVersion + "/accountusageinfo").HandlerFunc(httpTraceAll(adminAPI.AccountUsageInfoHandler))
+			adminRouter.Methods(http.MethodGet).Path(adminVersion + "/accountinfo").HandlerFunc(httpTraceAll(adminAPI.AccountInfoHandler))

 			adminRouter.Methods(http.MethodPut).Path(adminVersion+"/add-user").HandlerFunc(httpTraceHdrs(adminAPI.AddUser)).Queries("accessKey", "{accessKey:.*}")

--- a/cmd/api-errors.go
+++ b/cmd/api-errors.go
@ -363,6 +363,7 @@ const (
 	ErrInvalidDecompressedSize
 	ErrAddUserInvalidArgument
 	ErrAdminAccountNotEligible
+	ErrAccountNotEligible
 	ErrServiceAccountNotFound
 	ErrPostPolicyConditionInvalidFormat
 )
@ -1726,12 +1727,17 @@ var errorCodes = errorCodeMap{
 	ErrAddUserInvalidArgument: {
 		Code:           "XMinioInvalidIAMCredentials",
 		Description:    "User is not allowed to be same as admin access key",
-		HTTPStatusCode: http.StatusConflict,
+		HTTPStatusCode: http.StatusForbidden,
 	},
 	ErrAdminAccountNotEligible: {
 		Code:           "XMinioInvalidIAMCredentials",
 		Description:    "The administrator key is not eligible for this operation",
-		HTTPStatusCode: http.StatusConflict,
+		HTTPStatusCode: http.StatusForbidden,
+	},
+	ErrAccountNotEligible: {
+		Code:           "XMinioInvalidIAMCredentials",
+		Description:    "The account key is not eligible for this operation",
+		HTTPStatusCode: http.StatusForbidden,
 	},
 	ErrServiceAccountNotFound: {
 		Code:           "XMinioInvalidIAMCredentials",
--- a/cmd/iam.go
+++ b/cmd/iam.go
@ -1827,6 +1827,33 @@ func (sys *IAMSys) IsAllowedSTS(args iampolicy.Args) bool {
 	return combinedPolicy.IsAllowed(args)
 }

+// GetCombinedPolicy returns a combined policy combining all policies
+func (sys *IAMSys) GetCombinedPolicy(policies ...string) iampolicy.Policy {
+	// Policies were found, evaluate all of them.
+	sys.store.rlock()
+	defer sys.store.runlock()
+
+	var availablePolicies []iampolicy.Policy
+	for _, pname := range policies {
+		p, found := sys.iamPolicyDocsMap[pname]
+		if found {
+			availablePolicies = append(availablePolicies, p)
+		}
+	}
+
+	if len(availablePolicies) == 0 {
+		return iampolicy.Policy{}
+	}
+
+	combinedPolicy := availablePolicies[0]
+	for i := 1; i < len(availablePolicies); i++ {
+		combinedPolicy.Statements = append(combinedPolicy.Statements,
+			availablePolicies[i].Statements...)
+	}
+
+	return combinedPolicy
+}
+
 // IsAllowed - checks given policy args is allowed to continue the Rest API.
 func (sys *IAMSys) IsAllowed(args iampolicy.Args) bool {
 	// If opa is configured, use OPA always.
@ -1873,25 +1900,7 @@ func (sys *IAMSys) IsAllowed(args iampolicy.Args) bool {
 	}

 	// Policies were found, evaluate all of them.
-	sys.store.rlock()
-	defer sys.store.runlock()
-
-	var availablePolicies []iampolicy.Policy
-	for _, pname := range policies {
-		p, found := sys.iamPolicyDocsMap[pname]
-		if found {
-			availablePolicies = append(availablePolicies, p)
-		}
-	}
-	if len(availablePolicies) == 0 {
-		return false
-	}
-	combinedPolicy := availablePolicies[0]
-	for i := 1; i < len(availablePolicies); i++ {
-		combinedPolicy.Statements = append(combinedPolicy.Statements,
-			availablePolicies[i].Statements...)
-	}
-	return combinedPolicy.IsAllowed(args)
+	return sys.GetCombinedPolicy(policies...).IsAllowed(args)
 }

 // Set default canned policies only if not already overridden by users.
--- a/pkg/madmin/README.md
+++ b/pkg/madmin/README.md
@ -47,7 +47,7 @@ func main() {
 |:------------------------------------|:-----------------------------------------|:-------------------|:--------------------------|
 | [`ServiceTrace`](#ServiceTrace)     | [`ServerInfo`](#ServerInfo)              | [`Heal`](#Heal)    | [`GetConfig`](#GetConfig) |
 | [`ServiceStop`](#ServiceStop)       | [`StorageInfo`](#StorageInfo)            |                    | [`SetConfig`](#SetConfig) |
-| [`ServiceRestart`](#ServiceRestart) | [`AccountUsageInfo`](#AccountUsageInfo)  |                    |                           |
+| [`ServiceRestart`](#ServiceRestart) | [`AccountInfo`](#AccountInfo)  |                    |                           |



@ -251,16 +251,16 @@ __Example__

 ```

-<a name="AccountUsageInfo"></a>
+<a name="AccountInfo"></a>

-### AccountUsageInfo(ctx context.Context) (AccountUsageInfo, error)
+### AccountInfo(ctx context.Context) (AccountInfo, error)

 Fetches accounting usage information for the current authenticated user

 | Param                          | Type                 | Description             |
 |--------------------------------|----------------------|-------------------------|
-| `AccountUsageInfo.AccountName` | _string_             | Account name.           |
-| `AccountUsageInfo.Buckets`     | _[]BucketUsageInfo_  | Bucket usage info.      |
+| `AccountInfo.AccountName` | _string_             | Account name.           |
+| `AccountInfo.Buckets`     | _[]BucketUsageInfo_  | Bucket usage info.      |


 | Param                      | Type            | Description                             |
@ -281,12 +281,12 @@ __Example__

 ```go

-   accountUsageInfo, err := madmClnt.AccountUsageInfo(context.Background())
+   accountInfo, err := madmClnt.AccountInfo(context.Background())
   if err != nil {
 	log.Fatalln(err)
   }

-   log.Println(accountUsageInfo)
+   log.Println(accountInfo)

 ```

--- a/pkg/madmin/examples/accounting-usage-info.go
+++ b/pkg/madmin/examples/accounting-usage-info.go
@ -37,10 +37,10 @@ func main() {
 		log.Fatalln(err)
 	}

-	accountUsageInfo, err := madmClnt.AccountUsageInfo(context.Background())
+	accountInfo, err := madmClnt.AccountInfo(context.Background())
 	if err != nil {
 		log.Fatalln(err)
 	}

-	log.Println(accountUsageInfo)
+	log.Println(accountInfo)
 }
--- a/pkg/madmin/user-commands.go
+++ b/pkg/madmin/user-commands.go
@ -44,37 +44,38 @@ type BucketUsageInfo struct {
 	Access  AccountAccess `json:"access"`
 }

-// AccountUsageInfo represents the account usage info of an
+// AccountInfo represents the account usage info of an
 // account across buckets.
-type AccountUsageInfo struct {
+type AccountInfo struct {
 	AccountName string
+	Policy      iampolicy.Policy
 	Buckets     []BucketUsageInfo
 }

-// AccountUsageInfo returns the usage info for the authenticating account.
-func (adm *AdminClient) AccountUsageInfo(ctx context.Context) (AccountUsageInfo, error) {
-	resp, err := adm.executeMethod(ctx, http.MethodGet, requestData{relPath: adminAPIPrefix + "/accountusageinfo"})
+// AccountInfo returns the usage info for the authenticating account.
+func (adm *AdminClient) AccountInfo(ctx context.Context) (AccountInfo, error) {
+	resp, err := adm.executeMethod(ctx, http.MethodGet, requestData{relPath: adminAPIPrefix + "/accountinfo"})
 	defer closeResponse(resp)
 	if err != nil {
-		return AccountUsageInfo{}, err
+		return AccountInfo{}, err
 	}

 	// Check response http status code
 	if resp.StatusCode != http.StatusOK {
-		return AccountUsageInfo{}, httpRespToErrorResponse(resp)
+		return AccountInfo{}, httpRespToErrorResponse(resp)
 	}

 	// Unmarshal the server's json response
-	var accountInfo AccountUsageInfo
+	var accountInfo AccountInfo

 	respBytes, err := ioutil.ReadAll(resp.Body)
 	if err != nil {
-		return AccountUsageInfo{}, err
+		return AccountInfo{}, err
 	}

 	err = json.Unmarshal(respBytes, &accountInfo)
 	if err != nil {
-		return AccountUsageInfo{}, err
+		return AccountInfo{}, err
 	}

 	return accountInfo, nil