xborder commented on code in PR #2651:
URL: https://github.com/apache/arrow-adbc/pull/2651#discussion_r2031559353


##########
go/adbc/driver/flightsql/flightsql_database.go:
##########
@@ -160,14 +162,78 @@ func (d *databaseImpl) SetOptions(cnOptions 
map[string]string) error {
        if p, ok := cnOptions[adbc.OptionKeyPassword]; ok {
                if d.hdrs.Len() > 0 {
                        return adbc.Error{
-                               Msg:  "Authorization header already provided, 
do not provide user/pass also",
+                               Msg:  "Authentication conflict: Use either 
Authorization header OR username/password parameter",
                                Code: adbc.StatusInvalidArgument,
                        }
                }
                d.pass = p
                delete(cnOptions, adbc.OptionKeyPassword)
        }
 
+       // if token exists it can by pass or apply token exchange
+       // else check oauth flow
+       if t, ok := cnOptions[adbc.OptionKeyToken]; ok {
+               if d.hdrs.Len() > 0 {
+                       return adbc.Error{
+                               Msg:  "Authentication conflict: Use either 
Authorization header OR token parameter",
+                               Code: adbc.StatusInvalidArgument,
+                       }
+               }
+
+               // if contains token. it can bypass or use token exchange
+               if flow, ok := cnOptions[OptionKeyOauthFlow]; ok {
+                       var flowVal int
+                       var err error
+                       if flowVal, err = strconv.Atoi(flow); err != nil || 
flowVal != TokenExchange {
+                               return adbc.Error{
+                                       Msg:  "unsupported option",
+                                       Code: adbc.StatusInvalidArgument,
+                               }
+                       }
+
+                       tokExchange, err := newTokenExchangeFlow(cnOptions)
+                       if err != nil {
+                               return err
+                       }
+                       d.oauthFlow = tokExchange
+                       delete(cnOptions, OptionKeyOauthFlow)
+               } else {
+                       d.token = t
+                       delete(cnOptions, adbc.OptionKeyToken)
+               }

Review Comment:
   I don't think so. There are 3 scenarios at play:
   * when the dev sets the auth header
   * when the dev sets the token
     * on top of it you choose to do the token exchange (if branch)
     * you can just choose to use the token as is (else branch)
   * when the dev triggers a oauth flow (e.g client credentials, authorizaton 
code). In this case no headers or tokens should be set because the purpose of 
triggering the flow its to obtain one
   
   Does it make sense? probably should've been clearer on the PR description



-- 
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: github-unsubscr...@arrow.apache.org

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

Reply via email to