Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Incorrect usage of the X-Iceberg-Access-Delegation header #146

Open
1 task done
dimas-b opened this issue Aug 12, 2024 · 1 comment
Open
1 task done

[BUG] Incorrect usage of the X-Iceberg-Access-Delegation header #146

dimas-b opened this issue Aug 12, 2024 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@dimas-b
Copy link
Contributor

dimas-b commented Aug 12, 2024

Is this a possible security vulnerability?

  • This is NOT a possible security vulnerability

Describe the bug

Iceberg REST spec defines the following two values for the X-Iceberg-Access-Delegation HTTP header:

  • vended-credentials
  • remote-signing

However, current Polaris examples / docs use the value of true, which does not match anything in the Iceberg REST spec.

Current code appears to treat any non-empty header as vended-credentials, which is not exactly correct.

To Reproduce

No response

Actual Behavior

No response

Expected Behavior

Additional context

No response

System information

No response

@dimas-b dimas-b added the bug Something isn't working label Aug 12, 2024
@dimas-b dimas-b self-assigned this Aug 12, 2024
@dennishuo
Copy link
Contributor

This was definitely an unfortunate oversight; good catch!

Luckily, since the bug wasn't that the request handlers actually expected to parse true from the String, just that they only checked !isNullOrEmpty to assume it meant vended-credentials, it's forward-compatible if anyone is using the "correct" syntax.

We should try to update docs as quickly as possible to reduce the degree to which X-Iceberg-Access-Delegation: true gets entrenched into various workflows or test scripts of anyone integrating with Polaris.

At this point it may already be prudent to have an option for still falling back to the vended-credentials behavior if none of the comma-separated list of supported mechanisms is recognized (including the string "true").

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants