Skip to content

Feature make oauth audience configurable #9839

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.

Sign up for GitHub

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

Merged
flyrain merged 10 commits into from
Mar 11, 2024
Merged

Feature make oauth audience configurable #9839

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
ea72231
make `audience` configurable, also make arrangement to pass all optio…
Feb 29, 2024
0f72cc1
refreshToken also needs audience, add tests
Feb 29, 2024
bd908d9
fix imports
Feb 29, 2024
7f4556c
make sure no breaking public api changes.
Feb 29, 2024
effc916
create a helper method to build optional param map from properties
Feb 29, 2024
0cc33c9
add more comments, convert TODO to comment.
Feb 29, 2024
23e7452
style fix
Feb 29, 2024
3d4987b
remove `scope` as it is already being populated, fixed tests.
Feb 29, 2024
523a6ea
add scope as well in the optionalOAuthParam map and use buildKeepingL…
Mar 8, 2024
47da67a
adress review comments and address review comments
Mar 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 25 additions & 3 deletions aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3V4RestSignerClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,11 @@ public String oauth2ServerUri() {
return properties().getOrDefault(OAuth2Properties.OAUTH2_SERVER_URI, ResourcePaths.tokens());
}

@Value.Lazy
public Map<String, String> optionalOAuthParams() {
return OAuth2Util.buildOptionalParam(properties());
}

/** A Bearer token supplier which will be used for interaction with the server. */
@Value.Default
public Supplier<String> token() {
Expand Down Expand Up @@ -207,7 +212,13 @@ private AuthSession authSession() {
token,
expiresAtMillis(properties()),
new AuthSession(
ImmutableMap.of(), token, null, credential(), SCOPE, oauth2ServerUri())));
ImmutableMap.of(),
token,
null,
credential(),
SCOPE,
oauth2ServerUri(),
optionalOAuthParams())));
Comment on lines +215 to +221
Copy link
Contributor

@flyrain flyrain Mar 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we extract this part so that the following one at line 231 can reuse it?

Copy link
Contributor Author

@himadripal himadripal Mar 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one in line 231, do not have token provided (token = null), we can still create a private method with token as param, but this one looks more readable. WDYT?

}

if (credentialProvided()) {
Expand All @@ -217,11 +228,22 @@ private AuthSession authSession() {
id -> {
AuthSession session =
new AuthSession(
ImmutableMap.of(), null, null, credential(), SCOPE, oauth2ServerUri());
ImmutableMap.of(),
null,
null,
credential(),
SCOPE,
oauth2ServerUri(),
optionalOAuthParams());
long startTimeMillis = System.currentTimeMillis();
OAuthTokenResponse authResponse =
OAuth2Util.fetchToken(
httpClient(), session.headers(), credential(), SCOPE, oauth2ServerUri());
httpClient(),
session.headers(),
credential(),
SCOPE,
oauth2ServerUri(),
optionalOAuthParams());
return AuthSession.fromTokenResponse(
httpClient(), tokenRefreshExecutor(), authResponse, startTimeMillis, session);
});
Expand Down
8 changes: 6 additions & 2 deletions core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,14 +179,16 @@ public void initialize(String name, Map<String, String> unresolved) {
OAuthTokenResponse authResponse;
String credential = props.get(OAuth2Properties.CREDENTIAL);
String scope = props.getOrDefault(OAuth2Properties.SCOPE, OAuth2Properties.CATALOG_SCOPE);
Map<String, String> optionalOAuthParams = OAuth2Util.buildOptionalParam(props);
String oauth2ServerUri =
props.getOrDefault(OAuth2Properties.OAUTH2_SERVER_URI, ResourcePaths.tokens());
try (RESTClient initClient = clientBuilder.apply(props)) {
Map<String, String> initHeaders =
RESTUtil.merge(configHeaders(props), OAuth2Util.authHeaders(initToken));
if (credential != null && !credential.isEmpty()) {
authResponse =
OAuth2Util.fetchToken(initClient, initHeaders, credential, scope, oauth2ServerUri);
OAuth2Util.fetchToken(
initClient, initHeaders, credential, scope, oauth2ServerUri, optionalOAuthParams);
Map<String, String> authHeaders =
RESTUtil.merge(initHeaders, OAuth2Util.authHeaders(authResponse.token()));
config = fetchConfig(initClient, authHeaders, props);
Expand All @@ -213,7 +215,9 @@ public void initialize(String name, Map<String, String> unresolved) {
this.paths = ResourcePaths.forCatalogProperties(mergedProps);

String token = mergedProps.get(OAuth2Properties.TOKEN);
this.catalogAuth = new AuthSession(baseHeaders, null, null, credential, scope, oauth2ServerUri);
this.catalogAuth =
new AuthSession(
baseHeaders, null, null, credential, scope, oauth2ServerUri, optionalOAuthParams);
if (authResponse != null) {
this.catalogAuth =
AuthSession.fromTokenResponse(
Expand Down
6 changes: 6 additions & 0 deletions core/src/main/java/org/apache/iceberg/rest/auth/OAuth2Properties.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,12 @@ private OAuth2Properties() {}
/** Additional scope for OAuth2. */
public static final String SCOPE = "scope";

/** Optional param audience for OAuth2. */
public static final String AUDIENCE = "audience";

/** Optional param resource for OAuth2. */
public static final String RESOURCE = "resource";

/** Scope for OAuth2 flows. */
public static final String CATALOG_SCOPE = "catalog";

Expand Down
Loading