CXH-1990: Show why Snowflake sign-in failed instead of a bare error#137
CXH-1990: Show why Snowflake sign-in failed instead of a bare error#137al-conductorone wants to merge 5 commits into
Conversation
Validate/ListUsers returned a bare "code = Unauthenticated desc = 401
Unauthorized", so an expired trial or rotated key was indistinguishable
from an auth-handling defect. Pass uhttp.WithErrorResponse so the SQL API
error body ({code, message}) is decoded and surfaced in both the returned
error and the connector debug log, while keeping codes.Unauthenticated.
No user data is added to connector-side error/log text.
Admin review noted the SnowflakeError type in user_rest.go already has the
same {code, message} shape and implements uhttp.ErrorResponse. Reuse it in
ListUsers rather than introducing a duplicate statementsAPIError.
Connector PR Review: CXH-1990: Show why Snowflake sign-in failed instead of a bare errorBlocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0 Review SummaryThe full PR diff was scanned for security and correctness. The change does two things: (1) surfaces Snowflake's response-body reason on auth failures by adding Security IssuesNone found. Correctness IssuesNone found. SuggestionsNone. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
…ls-with-a-bare-401-unauthorized
| users, err := d.Client.ListUsers(ctx, "", 1) | ||
| if err != nil { | ||
| return nil, err | ||
| return nil, fmt.Errorf("baton-snowflake: credential validation failed: %w", err) |
There was a problem hiding this comment.
[LOW · optional] nice surfacing of the real auth reason. small wording thing: "credential validation failed" also labels non-credential transport failures (net/DNS/5xx) as credential problems — the wrapped %w still carries the true cause, so it's just the label. optional: soften to something like "snowflake: validation request failed: %w".
| } | ||
|
|
||
| var opts []rs.ResourceOption | ||
| opts := []rs.ResourceOption{ |
There was a problem hiding this comment.
[LOW · heads-up] this moves profile/status/created_at to resource-level options only (dropping the deprecated trait-level copy). fine for consumers on baton-sdk v0.20.2+, just worth a glance that the ingesting platform reads the resource-level attrs and not the old trait fields. no code change needed if it does.
5 participants
Snowflake syncs that fail to authenticate now report the reason returned by Snowflake, instead of a generic "unauthorized" message, by reading the error detail from the failed request.