Skip to content

Commit

Permalink
Restore soft deleted users on authentication (#5549)
Browse files Browse the repository at this point in the history 
* internal/clientcache: restore user on login

A user that previously had its last auth token deleted
(for example, if they logged out), should be restored
again when they successfully authenticate again.

* internal/clientcache: delete keyringless expired auth tokens

Previously, we would only correctly delete auth tokens
that were expired (or deleted) if they were also in the
keyring.

* internal/clientcache: remove tautological condition

At this stage we already know that at is non-nil, because
the case above would have been true if it was nil.

* internal/clientcache: improve error handling

The previous error handling would have ignored API errors that were not ErrUnauthorized or ErrNotFound.
Handle all API errors the same as other unexpected errors.

* internal/clientcache: make errors distinct

These two error conditions are otherwise impossible
to tell apart.

* test(e2e): Update search test to catch bug (#5551)

---------

Co-authored-by: Michael Li <michael.li@hashicorp.com>
  • Loading branch information
@johanbrandhorst @moduli
johanbrandhorst and moduli authored Feb 24, 2025
1 parent 11c19d5 commit 262693f
Show file tree
Hide file tree
Showing 3 changed files with 137 additions and 9 deletions.
16 changes: 8 additions & 8 deletions internal/clientcache/internal/cache/refresh.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,17 +94,16 @@ func (r *RefreshService) cleanAndPickAuthTokens(ctx context.Context, u *user) (m
if err := r.repo.deleteKeyringToken(ctx, *kt); err != nil {
return nil, errors.Wrap(ctx, err, op, errors.WithMsg("for user %q, auth token %q", u.Id, t.Id))
}
case at != nil:
default:
_, err := r.repo.tokenReadFromBoundaryFn(ctx, u.Address, at.Token)
var apiErr *api.Error
switch {
case err != nil && (api.ErrUnauthorized.Is(err) || api.ErrNotFound.Is(err)):
if err := r.repo.deleteKeyringToken(ctx, *kt); err != nil {
return nil, errors.Wrap(ctx, err, op, errors.WithMsg("for user %q, auth token %q", u.Id, t.Id))
}
event.WriteSysEvent(ctx, op, "Removed auth token from cache because it was not found to be valid in boundary", "auth token id", at.Id)
event.WriteSysEvent(ctx, op, "Removed auth token from db because it was not found to be valid in boundary", "auth token id", at.Id)
continue
case err != nil && !errors.Is(err, apiErr):
case err != nil:
event.WriteError(ctx, op, err, event.WithInfoMsg("validating keyring stored token against boundary", "auth token id", at.Id))
continue
}
Expand All @@ -114,17 +113,18 @@ func (r *RefreshService) cleanAndPickAuthTokens(ctx context.Context, u *user) (m
if atv, ok := r.repo.idToKeyringlessAuthToken.Load(t.Id); ok {
if at, ok := atv.(*authtokens.AuthToken); ok {
_, err := r.repo.tokenReadFromBoundaryFn(ctx, u.Address, at.Token)
var apiErr *api.Error
switch {
case err != nil && (api.ErrUnauthorized.Is(err) || api.ErrNotFound.Is(err)):
r.repo.idToKeyringlessAuthToken.Delete(t.Id)
event.WriteSysEvent(ctx, op, "Removed auth token from cache because it was not found to be valid in boundary", "auth token id", at.Id)
event.WriteSysEvent(ctx, op, "Removed auth token from in memory cache because it was not found to be valid in boundary", "auth token id", at.Id)
if err := r.repo.cleanExpiredOrOrphanedAuthTokens(ctx); err != nil {
return nil, errors.Wrap(ctx, err, op, errors.WithMsg("for user %q, auth token %q", u.Id, t.Id))
}
continue
case err != nil && !errors.Is(err, apiErr):
case err != nil:
event.WriteError(ctx, op, err, event.WithInfoMsg("validating in memory stored token against boundary", "auth token id", at.Id))
continue
}

ret[*t] = at.Token
}
}
Expand Down
3 changes: 2 additions & 1 deletion internal/clientcache/internal/cache/repository_token.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ func upsertUserAndAuthToken(ctx context.Context, reader db.Reader, writer db.Wri
}
onConflict := &db.OnConflict{
Target: db.Columns{"id"},
Action: db.DoNothing(true),
// Unset the deleted_at column if it was set to un-delete the user
Action: db.SetColumnValues(map[string]any{"deleted_at": "infinity"}),
}
if err := writer.Create(ctx, u, db.WithOnConflict(onConflict)); err != nil {
return errors.Wrap(ctx, err, op)
Expand Down
127 changes: 127 additions & 0 deletions testing/internal/e2e/tests/base/search_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -285,4 +285,131 @@ func TestCliSearch(t *testing.T) {
err = json.Unmarshal(output.Stdout, &searchResult)
require.NoError(t, err)
require.Len(t, searchResult.Item.Sessions, 0)

// Log out and confirm search does not work
output = e2e.RunCommand(ctx, "boundary", e2e.WithArgs("logout"))
require.NoError(t, output.Err, string(output.Stderr))
output = e2e.RunCommand(ctx, "boundary",
e2e.WithArgs(
"search",
"-resource", "targets",
"-format", "json",
"-query", fmt.Sprintf(`name %% "%s" and scope_id = "%s"`, targetPrefix, projectId),
),
)
require.Error(t, output.Err)

// Log back in and confirm search works
boundary.AuthenticateAdminCli(t, ctx)
output = e2e.RunCommand(ctx, "boundary",
e2e.WithArgs(
"search",
"-resource", "targets",
"-format", "json",
"-query", fmt.Sprintf(`name %% "%s" and scope_id = "%s"`, targetPrefix, projectId),
),
)
require.NoError(t, output.Err, string(output.Stderr))
searchResult = clientcache.SearchResult{}
err = json.Unmarshal(output.Stdout, &searchResult)
require.NoError(t, err)
require.Len(t, searchResult.Item.Targets, len(targetIds))

// Restart cache and confirm search works
t.Log("Restarting cache...")
output = e2e.RunCommand(ctx, "boundary", e2e.WithArgs("cache", "stop"))
require.NoError(t, output.Err, string(output.Stderr))
output = e2e.RunCommand(ctx, "boundary",
e2e.WithArgs(
"cache", "start",
"-refresh-interval", "5s",
"-background",
),
)
require.NoError(t, output.Err, string(output.Stderr))
err = backoff.RetryNotify(
func() error {
output := e2e.RunCommand(ctx, "boundary", e2e.WithArgs("cache", "status", "-format", "json"))
if output.Err != nil {
return errors.New(strings.TrimSpace(string(output.Stderr)))
}

err = json.Unmarshal(output.Stdout, &statusResult)
if err != nil {
return backoff.Permanent(err)
}

return nil
},
backoff.WithMaxRetries(backoff.NewConstantBackOff(1*time.Second), 5),
func(err error, td time.Duration) {
t.Logf("%s. Retrying...", err.Error())
},
)
require.NoError(t, err)
require.Equal(t, statusResult.StatusCode, 200)
require.GreaterOrEqual(t, statusResult.Item.Uptime, 0*time.Second)
output = e2e.RunCommand(ctx, "boundary",
e2e.WithArgs(
"search",
"-resource", "targets",
"-format", "json",
"-query", fmt.Sprintf(`name %% "%s" and scope_id = "%s"`, targetPrefix, projectId),
),
)
require.NoError(t, output.Err, string(output.Stderr))
searchResult = clientcache.SearchResult{}
err = json.Unmarshal(output.Stdout, &searchResult)
require.NoError(t, err)
require.Len(t, searchResult.Item.Targets, len(targetIds))

// Log out and restart cache. Log in and confirm search works
output = e2e.RunCommand(ctx, "boundary", e2e.WithArgs("logout"))
t.Log("Restarting cache...")
output = e2e.RunCommand(ctx, "boundary", e2e.WithArgs("cache", "stop"))
require.NoError(t, output.Err, string(output.Stderr))
output = e2e.RunCommand(ctx, "boundary",
e2e.WithArgs(
"cache", "start",
"-refresh-interval", "5s",
"-background",
),
)
require.NoError(t, output.Err, string(output.Stderr))
err = backoff.RetryNotify(
func() error {
output := e2e.RunCommand(ctx, "boundary", e2e.WithArgs("cache", "status", "-format", "json"))
if output.Err != nil {
return errors.New(strings.TrimSpace(string(output.Stderr)))
}

err = json.Unmarshal(output.Stdout, &statusResult)
if err != nil {
return backoff.Permanent(err)
}

return nil
},
backoff.WithMaxRetries(backoff.NewConstantBackOff(1*time.Second), 5),
func(err error, td time.Duration) {
t.Logf("%s. Retrying...", err.Error())
},
)
require.NoError(t, err)
require.Equal(t, statusResult.StatusCode, 200)
require.GreaterOrEqual(t, statusResult.Item.Uptime, 0*time.Second)
boundary.AuthenticateAdminCli(t, ctx)
output = e2e.RunCommand(ctx, "boundary",
e2e.WithArgs(
"search",
"-resource", "targets",
"-format", "json",
"-query", fmt.Sprintf(`name %% "%s" and scope_id = "%s"`, targetPrefix, projectId),
),
)
require.NoError(t, output.Err, string(output.Stderr))
searchResult = clientcache.SearchResult{}
err = json.Unmarshal(output.Stdout, &searchResult)
require.NoError(t, err)
require.Len(t, searchResult.Item.Targets, len(targetIds))
}

0 comments on commit 262693f

Please sign in to comment.