github-actions[bot] commented on code in PR #63068:
URL: https://github.com/apache/doris/pull/63068#discussion_r3323298152
##########
fe/fe-core/src/main/java/org/apache/doris/mysql/authenticate/AuthenticatorManager.java:
##########
@@ -240,7 +244,38 @@ private Optional<AuthenticateRequest>
resolveAuthenticateRequest(Authenticator a
private AuthenticateResponse authenticateWith(Authenticator authenticator,
AuthenticateRequest request) throws IOException {
- return authenticator.authenticate(request);
+ AuthenticateResponse response = authenticator.authenticate(request);
+ attachDelegatedCredential(response, request);
+ return response;
+ }
+
+ private void attachDelegatedCredential(AuthenticateResponse response,
AuthenticateRequest request) {
+ if (!response.isSuccess() || request.getCredential() == null ||
response.getDelegatedCredential() != null) {
+ return;
+ }
+ DelegatedCredential.Type type =
delegatedCredentialType(request.getCredentialType());
+ if (type == null) {
+ return;
+ }
+ OptionalLong expiresAtMillis = response.getCredentialExpiresAtMillis();
+ response.setDelegatedCredential(new DelegatedCredential(type,
+ new String(request.getCredential(), StandardCharsets.UTF_8),
expiresAtMillis));
+ }
+
+ private DelegatedCredential.Type delegatedCredentialType(String
credentialType) {
+ if (CredentialType.OAUTH_TOKEN.equals(credentialType)) {
+ return DelegatedCredential.Type.ACCESS_TOKEN;
Review Comment:
`CredentialType.OAUTH_TOKEN` is also what the MySQL
`authentication_openid_connect_client` path emits for an OIDC JWT
(`MysqlAuthPacketCredentialExtractor` and `normalizeAuthenticationChainRequest`
both produce it). This new mapping therefore stores those ID tokens as
`ACCESS_TOKEN`. With an Iceberg REST catalog using
`iceberg.rest.oauth2.delegated-token-mode=token_exchange`,
`IcebergSessionCatalogAdapter` will send the token as an OAuth access token
instead of `OAuth2Properties.ID_TOKEN_TYPE`, so REST token exchange receives
the wrong credential type. Please preserve whether the original delegated
credential is an OIDC ID token, or have the authenticator/plugin set the
`DelegatedCredential` explicitly instead of mapping every `OAUTH_TOKEN` request
to `ACCESS_TOKEN`.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergExternalCatalog.java:
##########
@@ -133,6 +141,90 @@ protected void initLocalObjectsImpl() {
metadataOps = ops;
}
+ @Override
+ protected List<String> listDatabaseNames() {
+ return super.listDatabaseNames();
+ }
+
+ protected List<String> listDatabaseNames(SessionContext ctx) {
+ if (isIcebergRestUserSessionPropertyEnabled()) {
+ return ((IcebergMetadataOps) metadataOps).listDatabaseNames(ctx);
+ }
+ return super.listDatabaseNames();
+ }
+
+ @Override
+ public List<String> getDbNames() {
+ if (isIcebergRestUserSessionPropertyEnabled()) {
+ makeSureInitialized();
+ return listDatabaseNames(currentSessionContext());
+ }
+ return super.getDbNames();
+ }
+
+ @Override
+ public List<String> getDbNamesOrEmpty() {
+ if (isIcebergRestUserSessionPropertyEnabled()) {
+ try {
+ return getDbNames();
+ } catch (Exception e) {
+ LOG.warn("failed to get db names in catalog {}", getName(), e);
+ return Collections.emptyList();
+ }
+ }
+ return super.getDbNamesOrEmpty();
+ }
+
+ @Override
+ public ExternalDatabase<? extends ExternalTable> getDbNullable(String
dbName) {
+ if (dbName == null || dbName.isEmpty() || isSystemDatabase(dbName)) {
+ return super.getDbNullable(dbName);
+ }
+ SessionContext ctx = currentSessionContext();
+ if (!ctx.hasDelegatedCredential()) {
+ return super.getDbNullable(dbName);
+ }
+ try {
+ makeSureInitialized();
+ } catch (Exception e) {
+ LOG.warn("failed to get db {} in catalog {}", dbName, getName(),
e);
+ return null;
+ }
+ if (!isIcebergRestUserSessionEnabled()) {
+ return super.getDbNullable(dbName);
+ }
+ return getDbNullableWithoutCache(ctx, dbName);
+ }
+
+ private ExternalDatabase<? extends ExternalTable>
getDbNullableWithoutCache(SessionContext ctx,
+ String remoteDbName) {
+ if (!databaseExists(ctx, remoteDbName)) {
+ return null;
Review Comment:
This cache-bypass path treats the caller's `dbName` as the remote Iceberg
namespace, but callers pass Doris local database names. That drops the existing
`fromRemoteDatabaseName` / `lower_case_database_names` resolution that
`ExternalCatalog.getDbNullable()` gets from the shared database cache. For
example, with remote namespace `SalesDB` and `lower_case_database_names=1`,
Doris exposes local db `salesdb`; a delegated request for `salesdb` now calls
`databaseExists(ctx, "salesdb")` and returns null even though
`listDatabaseNames(ctx)` would return `SalesDB`. The bypass should first
resolve the requested local name against the session-aware database list using
the same remote-to-local mapping rules, then use the matched remote name for
`namespaceExists` and `buildDbForInit`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
