[
https://issues.apache.org/jira/browse/DRILL-8223?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17538396#comment-17538396
]
ASF GitHub Bot commented on DRILL-8223:
---------------------------------------
cgivre commented on code in PR #2547:
URL: https://github.com/apache/drill/pull/2547#discussion_r875147774
##########
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/OAuthRequests.java:
##########
@@ -121,26 +106,19 @@ public static Response updateRefreshToken(String name,
OAuthTokenContainer token
public static Response updateOAuthTokens(String name, OAuthTokenContainer
tokenContainer, StoragePluginRegistry storage,
UserAuthEnabled authEnabled,
SecurityContext sc) {
try {
- if (storage.getPlugin(name).getConfig() instanceof
CredentialedStoragePluginConfig) {
Review Comment:
@jnturton Thanks for this PR. These functions used to have some checks to
see if the storage plugin in question was using OAuth. The prerequisite was
that the plugin used the `CredentialedStoragePluginConfig`.
Now that is no longer an option, do you think it's worth adding some check
to see if the plugin is in fact using OAuth? If not, throw some sort of error
saying you can't update tokens on a non-OAuth plugin?
In theory a user would never really be given that choice because the UI
takes care of that for them so I'm not sure if it is really necessary or not.
> Refactor auth modes dropping DRILL_PROCESS and allowing credential providers
> everywhere
> ---------------------------------------------------------------------------------------
>
> Key: DRILL-8223
> URL: https://issues.apache.org/jira/browse/DRILL-8223
> Project: Apache Drill
> Issue Type: Improvement
> Components: Security
> Affects Versions: 2.0.0
> Reporter: James Turton
> Assignee: James Turton
> Priority: Major
> Fix For: 2.0.0
>
>
> Remove the abstract CredentialedStoragePluginConfig (formerly
> AbstractSecuredStoragePluginConfig) class and promote the credential provider
> members to the parent StoragePluginConfig. Since having a credential provider
> provider is optional, there is no harm in giving the capability to every
> plugin's config.
> Drop the DRILL_PROCESS auth mode since this case is adequately covered by
> using SHARED_USER with no credentials specified.
> Bug fix. In storage-jdbc and when there are no JDBC credentials for the query
> user, only forgo an attempt to connect if the auth mode is USER_TRANSLATION.
> If it is SHARED_USER, proceed with an attempt to connect (examples of this
> case are unsecured DBs and BigQuery which requires OAuth tokens in the JDBC
> URL instead of a JDBC username and password).
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
