From 412018fbb41c206977cc5a40de3d8dda50352945 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Tue, 13 Apr 2021 16:55:40 -0500 Subject: [PATCH] Update misleading Authenticated bool with new name and explanation. (#1111) --- internal/auth/auth.go | 36 ++++++++++++++----- internal/perms/acl.go | 4 +-- .../authmethods/authmethod_service.go | 2 +- .../handlers/authtokens/authtoken_service.go | 2 +- .../handlers/groups/group_service.go | 2 +- .../host_catalogs/host_catalog_service.go | 2 +- .../controller/handlers/roles/role_service.go | 2 +- .../handlers/scopes/scope_service.go | 2 +- .../handlers/sessions/session_service.go | 2 +- .../handlers/targets/target_service.go | 2 +- .../controller/handlers/users/user_service.go | 2 +- 11 files changed, 39 insertions(+), 19 deletions(-) diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 9f8cbc7de5..7b213b9662 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -63,11 +63,31 @@ type RequestInfo struct { } type VerifyResults struct { - UserId string - AuthTokenId string - Error error - Scope *scopes.ScopeInfo - Authenticated bool + UserId string + AuthTokenId string + Error error + Scope *scopes.ScopeInfo + + // AuthenticatedFinished means that the request has passed through the + // authentication system successfully. This does _not_ indicate whether a + // token was provided on the request. Requests for `u_anon` will still have + // this set true! This is because if a request has a token that is invalid, + // we fall back to `u_anon` because the request may still be allowed for any + // anonymous user; it simply fails to validate for and look up grants for an + // actual known user. + // + // A good example is when running dev mode twice. The first time you can + // authenticate and get a token which is saved by the token helper. The + // second time, you run a command and it reads the token from the helper. + // That token is no longer valid, but if the action is granted to `u_anon` + // the action should still succeed. What happens internally is that the + // token fails to look up a non-anonymous user, so we fallback to the + // anonymous user, which is the default. + // + // If you want to know if the request had a valid token provided, use a + // switch on UserId. Anything that isn't `u_anon` will have to have had a + // valid token provided. And a valid token will never fall back to `u_anon`. + AuthenticationFinished bool // RoundTripValue can be set to allow the function performing authentication // (often accompanied by lookup(s)) to return a result of that lookup to the @@ -208,7 +228,7 @@ func Verify(ctx context.Context, opt ...Option) (ret VerifyResults) { } ret.AuthTokenId = v.requestInfo.PublicId - ret.Authenticated = authResults.Authenticated + ret.AuthenticationFinished = authResults.AuthenticationFinished if !authResults.Authorized { if v.requestInfo.DisableAuthzFailures { ret.Error = nil @@ -441,7 +461,7 @@ func (v verifier) performAuthCheck() (aclResults perms.ACLResults, userId string // At this point we don't need to look up grants since it's automatically allowed if v.requestInfo.TokenFormat == AuthTokenTypeRecoveryKms { - aclResults.Authenticated = true + aclResults.AuthenticationFinished = true aclResults.Authorized = true retErr = nil return @@ -481,7 +501,7 @@ func (v verifier) performAuthCheck() (aclResults perms.ACLResults, userId string // is used for further permissions checks, such as during recursive listing. // So we want to make sure any code relying on that has the full set of // grants successfully loaded. - aclResults.Authenticated = true + aclResults.AuthenticationFinished = true retErr = nil return } diff --git a/internal/perms/acl.go b/internal/perms/acl.go index 83c4171533..1f7bb5cac9 100644 --- a/internal/perms/acl.go +++ b/internal/perms/acl.go @@ -30,8 +30,8 @@ type ACL struct { // pass more detailed information along in the future if we want. It was useful // in Vault, may be useful here. type ACLResults struct { - Authenticated bool - Authorized bool + AuthenticationFinished bool + Authorized bool // This is included but unexported for testing/debugging scopeMap map[string][]Grant diff --git a/internal/servers/controller/handlers/authmethods/authmethod_service.go b/internal/servers/controller/handlers/authmethods/authmethod_service.go index d5e56dc678..022c56c7af 100644 --- a/internal/servers/controller/handlers/authmethods/authmethod_service.go +++ b/internal/servers/controller/handlers/authmethods/authmethod_service.go @@ -133,7 +133,7 @@ func (s Service) ListAuthMethods(ctx context.Context, req *pbs.ListAuthMethodsRe // may have authorization on downstream scopes. if authResults.Error == handlers.ForbiddenError() && req.GetRecursive() && - authResults.Authenticated { + authResults.AuthenticationFinished { } else { return nil, authResults.Error } diff --git a/internal/servers/controller/handlers/authtokens/authtoken_service.go b/internal/servers/controller/handlers/authtokens/authtoken_service.go index 051918d49f..f68ca9cfda 100644 --- a/internal/servers/controller/handlers/authtokens/authtoken_service.go +++ b/internal/servers/controller/handlers/authtokens/authtoken_service.go @@ -66,7 +66,7 @@ func (s Service) ListAuthTokens(ctx context.Context, req *pbs.ListAuthTokensRequ // may have authorization on downstream scopes. if authResults.Error == handlers.ForbiddenError() && req.GetRecursive() && - authResults.Authenticated { + authResults.AuthenticationFinished { } else { return nil, authResults.Error } diff --git a/internal/servers/controller/handlers/groups/group_service.go b/internal/servers/controller/handlers/groups/group_service.go index 7652c6f7aa..ec38a9b111 100644 --- a/internal/servers/controller/handlers/groups/group_service.go +++ b/internal/servers/controller/handlers/groups/group_service.go @@ -80,7 +80,7 @@ func (s Service) ListGroups(ctx context.Context, req *pbs.ListGroupsRequest) (*p // may have authorization on downstream scopes. if authResults.Error == handlers.ForbiddenError() && req.GetRecursive() && - authResults.Authenticated { + authResults.AuthenticationFinished { } else { return nil, authResults.Error } diff --git a/internal/servers/controller/handlers/host_catalogs/host_catalog_service.go b/internal/servers/controller/handlers/host_catalogs/host_catalog_service.go index 21a4bcdcc3..748b75b46e 100644 --- a/internal/servers/controller/handlers/host_catalogs/host_catalog_service.go +++ b/internal/servers/controller/handlers/host_catalogs/host_catalog_service.go @@ -116,7 +116,7 @@ func (s Service) ListHostCatalogs(ctx context.Context, req *pbs.ListHostCatalogs // may have authorization on downstream scopes. if authResults.Error == handlers.ForbiddenError() && req.GetRecursive() && - authResults.Authenticated { + authResults.AuthenticationFinished { } else { return nil, authResults.Error } diff --git a/internal/servers/controller/handlers/roles/role_service.go b/internal/servers/controller/handlers/roles/role_service.go index 0f22f383fe..b5c4e6ee2b 100644 --- a/internal/servers/controller/handlers/roles/role_service.go +++ b/internal/servers/controller/handlers/roles/role_service.go @@ -83,7 +83,7 @@ func (s Service) ListRoles(ctx context.Context, req *pbs.ListRolesRequest) (*pbs // may have authorization on downstream scopes. if authResults.Error == handlers.ForbiddenError() && req.GetRecursive() && - authResults.Authenticated { + authResults.AuthenticationFinished { } else { return nil, authResults.Error } diff --git a/internal/servers/controller/handlers/scopes/scope_service.go b/internal/servers/controller/handlers/scopes/scope_service.go index 3b0b2d96a1..902a8b9755 100644 --- a/internal/servers/controller/handlers/scopes/scope_service.go +++ b/internal/servers/controller/handlers/scopes/scope_service.go @@ -139,7 +139,7 @@ func (s Service) ListScopes(ctx context.Context, req *pbs.ListScopesRequest) (*p // may have authorization on downstream scopes. if authResults.Error == handlers.ForbiddenError() && req.GetRecursive() && - authResults.Authenticated { + authResults.AuthenticationFinished { } else { return nil, authResults.Error } diff --git a/internal/servers/controller/handlers/sessions/session_service.go b/internal/servers/controller/handlers/sessions/session_service.go index ead4c019cf..f10a78b00c 100644 --- a/internal/servers/controller/handlers/sessions/session_service.go +++ b/internal/servers/controller/handlers/sessions/session_service.go @@ -105,7 +105,7 @@ func (s Service) ListSessions(ctx context.Context, req *pbs.ListSessionsRequest) // may have authorization on downstream scopes. if authResults.Error == handlers.ForbiddenError() && req.GetRecursive() && - authResults.Authenticated { + authResults.AuthenticationFinished { } else { return nil, authResults.Error } diff --git a/internal/servers/controller/handlers/targets/target_service.go b/internal/servers/controller/handlers/targets/target_service.go index 45b8340115..63d35b877d 100644 --- a/internal/servers/controller/handlers/targets/target_service.go +++ b/internal/servers/controller/handlers/targets/target_service.go @@ -127,7 +127,7 @@ func (s Service) ListTargets(ctx context.Context, req *pbs.ListTargetsRequest) ( // may have authorization on downstream scopes. if authResults.Error == handlers.ForbiddenError() && req.GetRecursive() && - authResults.Authenticated { + authResults.AuthenticationFinished { } else { return nil, authResults.Error } diff --git a/internal/servers/controller/handlers/users/user_service.go b/internal/servers/controller/handlers/users/user_service.go index 1672538686..f760b3eff4 100644 --- a/internal/servers/controller/handlers/users/user_service.go +++ b/internal/servers/controller/handlers/users/user_service.go @@ -81,7 +81,7 @@ func (s Service) ListUsers(ctx context.Context, req *pbs.ListUsersRequest) (*pbs // may have authorization on downstream scopes. if authResults.Error == handlers.ForbiddenError() && req.GetRecursive() && - authResults.Authenticated { + authResults.AuthenticationFinished { } else { return nil, authResults.Error }