-
Notifications
You must be signed in to change notification settings - Fork 142
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
Do not URL-decode refresh header #332
Do not URL-decode refresh header #332
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a test for this in PolarisApplicationIntegrationTest
? We should be able to do something simple like send the snowmanCredentials with something like this
AuthConfig authConfig = Mockito.mock(AuthConfig.class)
// configure auth
RESTClient client = HttpClient.builder(props).uri(...).build();
AuthSession parentSession = new OAuth2Util.AuthSession(Map.of(), AuthConfig.builder().credential(snowmanCredentialsString).scope("PRINCIPAL_ROLE:ALL").build());
AuthSession.fromAccessToken(client, null, userToken, 0L, parentSession).refresh(client)
It won't guarantee that the characters are always ones that would be url-decoded, but sometimes they will be.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thanks for the fix. I don't know why I thought the credentials would be URL encoded...
Description
When the Iceberg client sends a request to refresh the auth token using a
Basic
header, the header is Base64-encoded but not URL-encoded.This discrepancy between the client's formatting of the header and the way Polaris is parsing the header can lead to certain values failing the authorization check.
Type of change
Please delete options that are not relevant.
Checklist:
Please delete options that are not relevant.