diff --git a/docs/databricks.md b/docs/databricks.md index e02fc74..30ddcaf 100644 --- a/docs/databricks.md +++ b/docs/databricks.md @@ -13,6 +13,10 @@ Secrets in Azure Key Vault-backed scopes are read through Databricks; if Databricks refuses, lazysecrets reads the backing vault directly with your az login (which then needs the [Key Vault roles](azure.md#permissions)). +When Databricks refuses, the pane says what to check: access to the +workspace and READ on the scope, or, when the workspace's IP access list +refused, the network you're on. + On a scope, the right pane shows who has access to it and how much (manage, write, read). Databricks shows that only to the scope's managers; to others it says so. diff --git a/internal/provider/azure/azure_test.go b/internal/provider/azure/azure_test.go index 62f783e..a2417f7 100644 --- a/internal/provider/azure/azure_test.go +++ b/internal/provider/azure/azure_test.go @@ -175,11 +175,11 @@ func TestForbiddenIsShortened(t *testing.T) { if err == nil { t.Fatal("expected error") } - if got, want := err.Error(), "403 Forbidden: Caller is not authorized to perform action on resource.\nCheck: have you activated your role or group in PIM?"; !strings.HasPrefix(got, want) { + if got, want := err.Error(), "403 Forbidden: Caller is not authorized to perform action on resource.\n\nCheck:\n• your role or group is activated in PIM\n• "; !strings.HasPrefix(got, want) { t.Errorf("error = %q, want %q…", got, want) } - if hint := deniedHint("Client address is not authorized and caller is not a trusted service."); !strings.Contains(hint, "network") { - t.Errorf("firewall hint = %q", hint) + if checks := deniedChecks("Client address is not authorized and caller is not a trusted service."); !strings.Contains(checks[0], "network") { + t.Errorf("firewall checks = %q", checks) } var re *azcore.ResponseError if !errors.As(err, &re) || re.StatusCode != 403 { diff --git a/internal/provider/azure/convert.go b/internal/provider/azure/convert.go index f29fca0..32f658c 100644 --- a/internal/provider/azure/convert.go +++ b/internal/provider/azure/convert.go @@ -280,7 +280,11 @@ func cleanError(err error) error { } } } - return &serviceError{status: re.StatusCode, msg: shortenDenied(msg), err: err} + se := &serviceError{status: re.StatusCode, msg: shortenDenied(msg), err: err} + if se.status == http.StatusForbidden { + return &provider.Denied{Err: se, Checks: deniedChecks(se.msg)} + } + return se } var policyDenied = regexp.MustCompile(`does not have (\w+ \w+) permission on key vault`) @@ -322,21 +326,23 @@ func (e *serviceError) Error() string { if e.msg != "" { s += ": " + e.msg } - if e.status == http.StatusForbidden { - s += "\n" + deniedHint(e.msg) - } return s } -// deniedHint is what to check when Key Vault refuses: the network, for -// its firewall's refusals, else access. On a line of its own, after the -// message. -func deniedHint(msg string) string { +// deniedChecks are what to check when Key Vault refuses: the network, +// for its firewall's refusals, else access. +func deniedChecks(msg string) []string { if strings.Contains(msg, "Client address is not authorized") || strings.Contains(msg, "network access") || strings.Contains(msg, "private link") { - return "Check: are you on a network the vault lets in (VPN, office)? Its firewall limits where it can be reached from." + return []string{ + "you're on a network the vault lets in (VPN, office)", + "the vault's owner has let your network in", + } + } + return []string{ + "your role or group is activated in PIM", + "the vault's owner has given you access: a Key Vault role, or its access policies", } - return "Check: have you activated your role or group in PIM? Has the vault's owner given you access (a Key Vault role, or its access policies)?" } func (e *serviceError) Unwrap() error { return e.err } diff --git a/internal/provider/databricks/databricks.go b/internal/provider/databricks/databricks.go index bb54095..8c86a4e 100644 --- a/internal/provider/databricks/databricks.go +++ b/internal/provider/databricks/databricks.go @@ -317,15 +317,47 @@ func (p *Provider) Value(ctx context.Context, o provider.Object) (provider.Secre return v, nil } +// deniedChecks are what to check when Databricks refuses: the network, +// when the workspace's IP access list refused, else access. +func deniedChecks(msg string) []string { + if strings.Contains(msg, "IP ACL") || strings.Contains(msg, "IP access list") { + return []string{ + "you're on a network the workspace lets in (VPN, office)", + "the workspace's admin has let your network in", + } + } + return []string{ + "you have access to the workspace", + "the scope's manager has given you READ on it, or a group you're in", + } +} + var errNoFallback = errors.New("no Key Vault fallback") -// combine reports both the Databricks and the Key Vault failure. An auth -// error from either stays reachable with errors.As. +// combine reports both the Databricks and the Key Vault failure, with +// both their checks after. An auth error from either stays reachable +// with errors.As. func combine(dbxErr, kvErr error) error { if errors.Is(kvErr, errNoFallback) { return dbxErr } - return fmt.Errorf("%w; Key Vault fallback: %w", dbxErr, kvErr) + var checks []string + strip := func(err error) error { + if d, ok := err.(*provider.Denied); ok { + for _, c := range d.Checks { + if !slices.Contains(checks, c) { + checks = append(checks, c) + } + } + return d.Err + } + return err + } + err := fmt.Errorf("%w; Key Vault fallback: %w", strip(dbxErr), strip(kvErr)) + if len(checks) > 0 { + return &provider.Denied{Err: err, Checks: checks} + } + return err } // classify turns authentication failures into *provider.AuthError: a 401 @@ -336,6 +368,9 @@ func classify(pr profile, err error) error { } var api *apierr.APIError if errors.As(err, &api) { + if api.StatusCode == 403 { + return &provider.Denied{Err: err, Checks: deniedChecks(api.Message)} + } if api.StatusCode != 401 { return err } diff --git a/internal/provider/databricks/databricks_test.go b/internal/provider/databricks/databricks_test.go index 0df6bf0..33efc6a 100644 --- a/internal/provider/databricks/databricks_test.go +++ b/internal/provider/databricks/databricks_test.go @@ -145,6 +145,8 @@ func workspaceAPI(t *testing.T, status int) *httptest.Server { map[string]any{"principal": "users", "permission": "READ"}, map[string]any{"principal": "admins", "permission": "MANAGE"}, }}) + case r.URL.Path == "/api/2.0/secrets/list" && q.Get("scope") == "locked": + write(w, 403, map[string]any{"error_code": "PERMISSION_DENIED", "message": "User does not have READ permission on scope locked"}) case r.URL.Path == "/api/2.0/secrets/acls/list" && q.Get("scope") == "locked": write(w, 403, map[string]any{"error_code": "PERMISSION_DENIED", "message": "User does not have MANAGE permission"}) case r.URL.Path == "/api/2.0/secrets/get" && q.Get("scope") == "app": @@ -315,3 +317,22 @@ func TestScopeDetails(t *testing.T) { t.Errorf("locked: %+v, %v", attrs, err) } } + +func TestDeniedSaysWhatToCheck(t *testing.T) { + p := newTestProvider(t, workspaceAPI(t, 0), nil) + _, err := p.Objects(context.Background(), provider.Container{Source: provider.Source{ID: "prod"}, Name: "locked"}, provider.TypeSecret) + var d *provider.Denied + if !errors.As(err, &d) || !strings.Contains(err.Error(), "READ permission on scope locked\n\nCheck:\n• you have access to the workspace") { + t.Errorf("err %q", err) + } + if got := deniedChecks("Source IP address: 1.2.3.4 is blocked by Databricks IP ACL for workspace: 1"); !strings.Contains(got[0], "network") { + t.Errorf("IP ACL checks %q", got) + } + + // Both refused: one message, then both lists of checks. + err = combine(&provider.Denied{Err: errors.New("dbx"), Checks: []string{"a", "b"}}, + &provider.Denied{Err: errors.New("kv"), Checks: []string{"b", "c"}}) + if got := err.Error(); got != "dbx; Key Vault fallback: kv\n\nCheck:\n• a\n• b\n• c" { + t.Errorf("combined %q", got) + } +} diff --git a/internal/provider/provider.go b/internal/provider/provider.go index 4e7dfab..c47dccc 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -12,6 +12,7 @@ import ( "context" "errors" "fmt" + "strings" "time" ) @@ -202,6 +203,23 @@ type Account struct { Copy, CopyLabel string } +// Denied is a refusal with what to check, from what the service said: a +// role to activate in PIM, access to ask for, the network. Error is the +// refusal, then the checks as a list. +type Denied struct { + Err error + Checks []string +} + +func (d *Denied) Error() string { + if len(d.Checks) == 0 { + return d.Err.Error() + } + return d.Err.Error() + "\n\nCheck:\n• " + strings.Join(d.Checks, "\n• ") +} + +func (d *Denied) Unwrap() error { return d.Err } + // ContainerDetails is implemented by providers that can say more about a // container than its listing does: a vault's settings, who has access to // a scope, how a file is encrypted and what git knows of it. It may go diff --git a/internal/ui/panes/list.go b/internal/ui/panes/list.go index aa685fd..0b9478f 100644 --- a/internal/ui/panes/list.go +++ b/internal/ui/panes/list.go @@ -334,7 +334,7 @@ func (l List[T]) View(st styles.Styles, focused bool, spin string) string { case l.Loading && len(l.items) == 0: body = append(body, " "+spin+" loading…") case l.Err != nil: - for _, line := range strings.Split(ansi.Wrap(l.Err.Error(), inner-1, " -/"), "\n") { + for _, line := range wrapLines(l.Err.Error(), inner-1) { body = append(body, st.Error.Render(" "+line)) } case len(l.visible) == 0 && len(l.items) > 0: @@ -393,3 +393,22 @@ func (l *List[T]) scroll() { } l.offset = max(0, min(l.offset, len(l.visible)-rows)) } + +// wrapLines wraps text to width, line by line; a "• " item's further +// lines line up under its text. +func wrapLines(text string, width int) []string { + var out []string + for _, line := range strings.Split(text, "\n") { + indent := "" + if strings.HasPrefix(line, "• ") { + indent = " " + } + for i, l := range strings.Split(ansi.Wrap(line, max(1, width-len(indent)), " -/"), "\n") { + if i > 0 { + l = indent + l + } + out = append(out, l) + } + } + return out +}