Fix browser authorization edge cases
This commit is contained in:
@@ -101,6 +101,9 @@ func (h *Host) Stop() error {
|
||||
h.started = false
|
||||
h.grpcServer.Stop()
|
||||
err := h.listener.Close()
|
||||
if errors.Is(err, net.ErrClosed) {
|
||||
err = nil
|
||||
}
|
||||
if h.socketPath != "" {
|
||||
if removeErr := os.Remove(h.socketPath); removeErr != nil && !errors.Is(removeErr, os.ErrNotExist) && err == nil {
|
||||
err = removeErr
|
||||
|
||||
@@ -348,22 +348,26 @@ func (s *Server) authorizedBrowserMatches(ctx context.Context, token apitokens.T
|
||||
if _, err := s.authorizeResourceRequest(ctx, token, apitokens.OperationListEntries, match.resource); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return browserMatchesWithinPath(matches, match.resource.Path), nil
|
||||
return s.authorizedBrowserMatchesWithinPath(ctx, token, matches, match.resource.Path)
|
||||
}
|
||||
return out, nil
|
||||
}
|
||||
|
||||
func browserMatchesWithinPath(matches []rankedBrowserMatch, path []string) []*keepassgov1.BrowserLoginMatch {
|
||||
func (s *Server) authorizedBrowserMatchesWithinPath(_ context.Context, _ apitokens.Token, matches []rankedBrowserMatch, path []string) ([]*keepassgov1.BrowserLoginMatch, error) {
|
||||
out := make([]*keepassgov1.BrowserLoginMatch, 0, len(matches))
|
||||
for _, match := range matches {
|
||||
if len(path) > len(match.resource.Path) {
|
||||
continue
|
||||
}
|
||||
if slices.Equal(path, match.resource.Path[:len(path)]) {
|
||||
out = append(out, match.match)
|
||||
if !slices.Equal(path, match.resource.Path[:len(path)]) {
|
||||
continue
|
||||
}
|
||||
if match.decision == apitokens.DecisionDeny {
|
||||
continue
|
||||
}
|
||||
out = append(out, match.match)
|
||||
}
|
||||
return out
|
||||
return out, nil
|
||||
}
|
||||
|
||||
func (s *Server) GetBrowserCredential(ctx context.Context, req *keepassgov1.GetBrowserCredentialRequest) (*keepassgov1.GetBrowserCredentialResponse, error) {
|
||||
@@ -1068,8 +1072,6 @@ func classifyBrowserEntryMatch(pageHost, rawEntryURL string) (string, int) {
|
||||
return "exact-host", 3
|
||||
case strings.HasSuffix(pageHost, "."+entryHost):
|
||||
return "subdomain", 2
|
||||
case strings.HasSuffix(entryHost, "."+pageHost):
|
||||
return "parent-domain", 1
|
||||
default:
|
||||
return "", 0
|
||||
}
|
||||
|
||||
@@ -336,6 +336,108 @@ func TestVaultServiceFindsBrowserLoginsWithinAuthorizedGroupScope(t *testing.T)
|
||||
}
|
||||
}
|
||||
|
||||
func TestVaultServiceFindsBrowserLoginsRechecksChildPoliciesAfterPrompt(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
model := vault.Model{
|
||||
Entries: []vault.Entry{
|
||||
{
|
||||
ID: "rusty-casino",
|
||||
Title: "Rusty Casino",
|
||||
Username: "rustyryan",
|
||||
Password: "bellagio-1",
|
||||
URL: "https://vault.heist.example.invalid",
|
||||
Path: []string{"Crews", "Bellagio"},
|
||||
},
|
||||
{
|
||||
ID: "benedict-vault",
|
||||
Title: "Benedict Vault",
|
||||
Username: "terrybenedict",
|
||||
Password: "bellagio-2",
|
||||
URL: "https://vault.heist.example.invalid",
|
||||
Path: []string{"Crews", "Bellagio", "Denied"},
|
||||
},
|
||||
testAPITokenEntry(t,
|
||||
apitokens.PolicyRule{Effect: apitokens.EffectDeny, Operation: apitokens.OperationListEntries, Resource: apitokens.Resource{Kind: apitokens.ResourceGroup, Path: []string{"Crews", "Bellagio", "Denied"}}},
|
||||
),
|
||||
},
|
||||
}
|
||||
client, _, service, cleanup := newTestHarnessForModel(t, model)
|
||||
defer cleanup()
|
||||
service.approvals = apiapproval.NewBroker(time.Minute)
|
||||
|
||||
respCh := make(chan *keepassgov1.FindBrowserLoginsResponse, 1)
|
||||
errCh := make(chan error, 1)
|
||||
go func() {
|
||||
resp, err := client.FindBrowserLogins(tokenContext(defaultTestTokenSecret), &keepassgov1.FindBrowserLoginsRequest{
|
||||
PageUrl: "https://vault.heist.example.invalid/login",
|
||||
})
|
||||
respCh <- resp
|
||||
errCh <- err
|
||||
}()
|
||||
|
||||
pending := waitForServerPendingApproval(t, service, 1)[0]
|
||||
if got := pending.Resource.Path; !slices.Equal(got, []string{"Crews", "Bellagio"}) {
|
||||
t.Fatalf("pending.Resource.Path = %v, want [Crews Bellagio]", got)
|
||||
}
|
||||
if _, _, err := service.ResolveApproval(pending.ID, apiapproval.OutcomeAllowOnce); err != nil {
|
||||
t.Fatalf("ResolveApproval(allow once) error = %v", err)
|
||||
}
|
||||
|
||||
resp := <-respCh
|
||||
if err := <-errCh; err != nil {
|
||||
t.Fatalf("FindBrowserLogins() error = %v", err)
|
||||
}
|
||||
if len(resp.Matches) != 1 {
|
||||
t.Fatalf("len(FindBrowserLogins().Matches) = %d, want 1", len(resp.Matches))
|
||||
}
|
||||
if got := resp.Matches[0].Id; got != "rusty-casino" {
|
||||
t.Fatalf("FindBrowserLogins().Matches[0].Id = %q, want rusty-casino", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestVaultServiceDoesNotMatchSpecificBrowserEntryToParentDomain(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
client, _, cleanup := newTestClientForModel(t, vault.Model{
|
||||
Entries: []vault.Entry{
|
||||
{
|
||||
ID: "inside-man-accounts",
|
||||
Title: "Inside Man Accounts",
|
||||
Username: "daltonrussell",
|
||||
Password: "diamond-1",
|
||||
URL: "https://accounts.heist.example.invalid",
|
||||
Path: []string{"Crews", "Bank"},
|
||||
},
|
||||
testAPITokenEntry(t,
|
||||
apitokens.PolicyRule{Effect: apitokens.EffectAllow, Operation: apitokens.OperationListEntries, Resource: apitokens.Resource{Kind: apitokens.ResourceGroup, Path: []string{"Crews"}}},
|
||||
apitokens.PolicyRule{Effect: apitokens.EffectAllow, Operation: apitokens.OperationCopyUsername, Resource: apitokens.Resource{Kind: apitokens.ResourceEntry, EntryID: "inside-man-accounts", Path: []string{"Crews", "Bank"}}},
|
||||
apitokens.PolicyRule{Effect: apitokens.EffectAllow, Operation: apitokens.OperationCopyPassword, Resource: apitokens.Resource{Kind: apitokens.ResourceEntry, EntryID: "inside-man-accounts", Path: []string{"Crews", "Bank"}}},
|
||||
apitokens.PolicyRule{Effect: apitokens.EffectAllow, Operation: apitokens.OperationCopyURL, Resource: apitokens.Resource{Kind: apitokens.ResourceEntry, EntryID: "inside-man-accounts", Path: []string{"Crews", "Bank"}}},
|
||||
),
|
||||
},
|
||||
})
|
||||
defer cleanup()
|
||||
|
||||
resp, err := client.FindBrowserLogins(tokenContext(defaultTestTokenSecret), &keepassgov1.FindBrowserLoginsRequest{
|
||||
PageUrl: "https://heist.example.invalid/login",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("FindBrowserLogins() error = %v", err)
|
||||
}
|
||||
if len(resp.Matches) != 0 {
|
||||
t.Fatalf("len(FindBrowserLogins().Matches) = %d, want 0", len(resp.Matches))
|
||||
}
|
||||
|
||||
_, err = client.GetBrowserCredential(tokenContext(defaultTestTokenSecret), &keepassgov1.GetBrowserCredentialRequest{
|
||||
Id: "inside-man-accounts",
|
||||
PageUrl: "https://heist.example.invalid/login",
|
||||
})
|
||||
if status.Code(err) != codes.InvalidArgument {
|
||||
t.Fatalf("GetBrowserCredential() code = %v, want %v", status.Code(err), codes.InvalidArgument)
|
||||
}
|
||||
}
|
||||
|
||||
func TestVaultServiceListEntriesHidesSingleInternalVaultRoot(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user