You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "findingrish (via GitHub)" <gi...@apache.org> on 2023/03/24 11:11:42 UTC

[GitHub] [druid] findingrish opened a new pull request, #13973: Update OIDCConfig with scope information

findingrish opened a new pull request, #13973:
URL: https://github.com/apache/druid/pull/13973

   Updates OIDCConfig class with scope information 
   Set scope for OIDCConfiguration object 
   
   This PR has:
   
   - [x] been self-reviewed.
   
   


-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] findingrish commented on a diff in pull request #13973: Update OIDCConfig with scope information

Posted by "findingrish (via GitHub)" <gi...@apache.org>.
findingrish commented on code in PR #13973:
URL: https://github.com/apache/druid/pull/13973#discussion_r1147458603


##########
extensions-core/druid-pac4j/src/main/java/org/apache/druid/security/pac4j/OIDCConfig.java:
##########
@@ -35,16 +35,21 @@
   @JsonProperty
   private final String discoveryURI;
 
+  @JsonProperty
+  private final String scope;
+
   @JsonCreator
   public OIDCConfig(
       @JsonProperty("clientID") String clientID,
       @JsonProperty("clientSecret") PasswordProvider clientSecret,
-      @JsonProperty("discoveryURI") String discoveryURI
+      @JsonProperty("discoveryURI") String discoveryURI,
+      @JsonProperty("scope") String scope
   )
   {
     this.clientID = Preconditions.checkNotNull(clientID, "null clientID");
     this.clientSecret = Preconditions.checkNotNull(clientSecret, "null clientSecret");
     this.discoveryURI = Preconditions.checkNotNull(discoveryURI, "null discoveryURI");
+    this.scope = Preconditions.checkNotNull(scope, "null scope");

Review Comment:
   thanks for pointing out, updated. 



-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekagarwal87 commented on a diff in pull request #13973: Update OIDCConfig with scope information

Posted by "abhishekagarwal87 (via GitHub)" <gi...@apache.org>.
abhishekagarwal87 commented on code in PR #13973:
URL: https://github.com/apache/druid/pull/13973#discussion_r1148373194


##########
docs/development/extensions-core/druid-pac4j.md:
##########


Review Comment:
   https://github.com/pac4j/pac4j/blob/master/pac4j-oidc/src/main/java/org/pac4j/oidc/config/OidcConfigurationContext.java#L70



-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekagarwal87 merged pull request #13973: Update OIDCConfig with scope information

Posted by "abhishekagarwal87 (via GitHub)" <gi...@apache.org>.
abhishekagarwal87 merged PR #13973:
URL: https://github.com/apache/druid/pull/13973


-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekagarwal87 commented on a diff in pull request #13973: Update OIDCConfig with scope information

Posted by "abhishekagarwal87 (via GitHub)" <gi...@apache.org>.
abhishekagarwal87 commented on code in PR #13973:
URL: https://github.com/apache/druid/pull/13973#discussion_r1147548691


##########
docs/development/extensions-core/druid-pac4j.md:
##########


Review Comment:
   default scope is "openid profile email" from what I can tell looking in the lib. 



-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekagarwal87 commented on pull request #13973: Update OIDCConfig with scope information

Posted by "abhishekagarwal87 (via GitHub)" <gi...@apache.org>.
abhishekagarwal87 commented on PR #13973:
URL: https://github.com/apache/druid/pull/13973#issuecomment-1482637625

   you may need to update the docs as well. 


-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] abhishekagarwal87 commented on a diff in pull request #13973: Update OIDCConfig with scope information

Posted by "abhishekagarwal87 (via GitHub)" <gi...@apache.org>.
abhishekagarwal87 commented on code in PR #13973:
URL: https://github.com/apache/druid/pull/13973#discussion_r1147437143


##########
extensions-core/druid-pac4j/src/main/java/org/apache/druid/security/pac4j/OIDCConfig.java:
##########
@@ -35,16 +35,21 @@
   @JsonProperty
   private final String discoveryURI;
 
+  @JsonProperty
+  private final String scope;
+
   @JsonCreator
   public OIDCConfig(
       @JsonProperty("clientID") String clientID,
       @JsonProperty("clientSecret") PasswordProvider clientSecret,
-      @JsonProperty("discoveryURI") String discoveryURI
+      @JsonProperty("discoveryURI") String discoveryURI,
+      @JsonProperty("scope") String scope
   )
   {
     this.clientID = Preconditions.checkNotNull(clientID, "null clientID");
     this.clientSecret = Preconditions.checkNotNull(clientSecret, "null clientSecret");
     this.discoveryURI = Preconditions.checkNotNull(discoveryURI, "null discoveryURI");
+    this.scope = Preconditions.checkNotNull(scope, "null scope");

Review Comment:
   null scope should be allowed or otherwise existing installations will break. 



-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] findingrish commented on a diff in pull request #13973: Update OIDCConfig with scope information

Posted by "findingrish (via GitHub)" <gi...@apache.org>.
findingrish commented on code in PR #13973:
URL: https://github.com/apache/druid/pull/13973#discussion_r1147612437


##########
docs/development/extensions-core/druid-pac4j.md:
##########


Review Comment:
   thanks for the feedback, I will update the docs. 
   
   Where did you find the default value? 



-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org