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


##########
go/adbc/driver/flightsql/flightsql_oauth.go:
##########
@@ -0,0 +1,149 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package flightsql
+
+import (
+       "context"
+       "fmt"
+
+       "golang.org/x/oauth2"
+       "google.golang.org/grpc/credentials"
+       "google.golang.org/grpc/credentials/oauth"
+)
+
+const (
+       ClientCredentials = "client_credentials"
+       TokenExchange     = "token_exchange"
+)
+
+type oAuthOption struct {
+       isRequired bool
+       oAuthKey   string
+}
+
+var (
+       clientCredentialsParams = map[string]oAuthOption{
+               OptionKeyClientId:     {true, "client_id"},
+               OptionKeyClientSecret: {true, "client_secret"},
+               OptionKeyTokenURI:     {true, "token_uri"},
+               OptionKeyScope:        {false, "scope"},
+       }
+
+       tokenExchangParams = map[string]oAuthOption{
+               OptionKeySubjectToken:     {true, "subject_token"},
+               OptionKeySubjectTokenType: {true, "subject_token_type"},
+               OptionKeyReqTokenType:     {false, "requested_token_type"},
+               OptionKeyExchangeAud:      {false, "audience"},
+               OptionKeyExchangeResource: {false, "resource"},
+               OptionKeyExchangeScope:    {false, "scope"},
+       }
+)
+
+func parseOAuthOptions(options map[string]string, paramMap 
map[string]oAuthOption, flowName string) (map[string]string, error) {
+       params := map[string]string{}
+
+       for key, param := range paramMap {
+               if value, ok := options[key]; ok {
+                       params[key] = value
+                       delete(options, key)
+               } else if param.isRequired {
+                       return nil, fmt.Errorf("%s grant requires %s", 
flowName, key)
+               }
+       }
+
+       return params, nil
+}
+
+func exchangeToken(conf *oauth2.Config, codeOptions []oauth2.AuthCodeOption) 
(credentials.PerRPCCredentials, error) {
+       ctx := context.Background()
+       tok, err := conf.Exchange(ctx, "", codeOptions...)
+       if err != nil {
+               return nil, err
+       }
+       return &oauth.TokenSource{TokenSource: conf.TokenSource(ctx, tok)}, nil
+}
+
+func newClientCredentials(options map[string]string) 
(credentials.PerRPCCredentials, error) {
+       codeOptions := []oauth2.AuthCodeOption{
+               oauth2.SetAuthURLParam("grant_type", "client_credentials"),
+       }
+
+       params, err := parseOAuthOptions(options, clientCredentialsParams, 
"client credentials")
+       if err != nil {
+               return nil, err
+       }
+
+       conf := &oauth2.Config{
+               ClientID:     params[OptionKeyClientId],
+               ClientSecret: params[OptionKeyClientSecret],
+               Endpoint: oauth2.Endpoint{
+                       TokenURL: params[OptionKeyTokenURI],
+               },
+       }
+
+       if scopes, ok := params[OptionKeyScope]; ok {
+               conf.Scopes = []string{scopes}
+       }
+
+       return exchangeToken(conf, codeOptions)
+}
+
+func newTokenExchangeFlow(options map[string]string) 
(credentials.PerRPCCredentials, error) {
+       tokenURI, ok := options[OptionKeyTokenURI]
+       if !ok {
+               return nil, fmt.Errorf("token exchange grant requires %s", 
OptionKeyTokenURI)
+       }
+       delete(options, OptionKeyTokenURI)
+
+       conf := &oauth2.Config{
+               Endpoint: oauth2.Endpoint{
+                       TokenURL: tokenURI,
+               },
+       }
+
+       codeOptions := []oauth2.AuthCodeOption{
+               oauth2.SetAuthURLParam("grant_type", 
"urn:ietf:params:oauth:grant-type:token-exchange"),

Review Comment:
   yes, it is specified in the 
[RFC8693](https://datatracker.ietf.org/doc/html/rfc8693#name-request)



##########
go/adbc/driver/flightsql/flightsql_oauth.go:
##########
@@ -0,0 +1,149 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package flightsql
+
+import (
+       "context"
+       "fmt"
+
+       "golang.org/x/oauth2"
+       "google.golang.org/grpc/credentials"
+       "google.golang.org/grpc/credentials/oauth"
+)
+
+const (
+       ClientCredentials = "client_credentials"
+       TokenExchange     = "token_exchange"
+)
+
+type oAuthOption struct {
+       isRequired bool
+       oAuthKey   string
+}
+
+var (
+       clientCredentialsParams = map[string]oAuthOption{
+               OptionKeyClientId:     {true, "client_id"},
+               OptionKeyClientSecret: {true, "client_secret"},
+               OptionKeyTokenURI:     {true, "token_uri"},
+               OptionKeyScope:        {false, "scope"},
+       }
+
+       tokenExchangParams = map[string]oAuthOption{
+               OptionKeySubjectToken:     {true, "subject_token"},
+               OptionKeySubjectTokenType: {true, "subject_token_type"},
+               OptionKeyReqTokenType:     {false, "requested_token_type"},
+               OptionKeyExchangeAud:      {false, "audience"},
+               OptionKeyExchangeResource: {false, "resource"},
+               OptionKeyExchangeScope:    {false, "scope"},
+       }
+)
+
+func parseOAuthOptions(options map[string]string, paramMap 
map[string]oAuthOption, flowName string) (map[string]string, error) {
+       params := map[string]string{}
+
+       for key, param := range paramMap {
+               if value, ok := options[key]; ok {
+                       params[key] = value
+                       delete(options, key)
+               } else if param.isRequired {
+                       return nil, fmt.Errorf("%s grant requires %s", 
flowName, key)
+               }
+       }
+
+       return params, nil
+}
+
+func exchangeToken(conf *oauth2.Config, codeOptions []oauth2.AuthCodeOption) 
(credentials.PerRPCCredentials, error) {
+       ctx := context.Background()
+       tok, err := conf.Exchange(ctx, "", codeOptions...)
+       if err != nil {
+               return nil, err
+       }
+       return &oauth.TokenSource{TokenSource: conf.TokenSource(ctx, tok)}, nil
+}
+
+func newClientCredentials(options map[string]string) 
(credentials.PerRPCCredentials, error) {
+       codeOptions := []oauth2.AuthCodeOption{
+               oauth2.SetAuthURLParam("grant_type", "client_credentials"),
+       }
+
+       params, err := parseOAuthOptions(options, clientCredentialsParams, 
"client credentials")
+       if err != nil {
+               return nil, err
+       }
+
+       conf := &oauth2.Config{
+               ClientID:     params[OptionKeyClientId],
+               ClientSecret: params[OptionKeyClientSecret],
+               Endpoint: oauth2.Endpoint{
+                       TokenURL: params[OptionKeyTokenURI],
+               },
+       }
+
+       if scopes, ok := params[OptionKeyScope]; ok {
+               conf.Scopes = []string{scopes}

Review Comment:
   I don't think so. the option value is already flattened because it is a 
string.  We can split by spaces but it will be flattened again for the request



##########
go/adbc/driver/flightsql/flightsql_oauth.go:
##########
@@ -0,0 +1,149 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package flightsql
+
+import (
+       "context"
+       "fmt"
+
+       "golang.org/x/oauth2"
+       "google.golang.org/grpc/credentials"
+       "google.golang.org/grpc/credentials/oauth"
+)
+
+const (
+       ClientCredentials = "client_credentials"
+       TokenExchange     = "token_exchange"
+)
+
+type oAuthOption struct {
+       isRequired bool
+       oAuthKey   string
+}
+
+var (
+       clientCredentialsParams = map[string]oAuthOption{
+               OptionKeyClientId:     {true, "client_id"},
+               OptionKeyClientSecret: {true, "client_secret"},
+               OptionKeyTokenURI:     {true, "token_uri"},
+               OptionKeyScope:        {false, "scope"},
+       }
+
+       tokenExchangParams = map[string]oAuthOption{
+               OptionKeySubjectToken:     {true, "subject_token"},
+               OptionKeySubjectTokenType: {true, "subject_token_type"},
+               OptionKeyReqTokenType:     {false, "requested_token_type"},
+               OptionKeyExchangeAud:      {false, "audience"},
+               OptionKeyExchangeResource: {false, "resource"},
+               OptionKeyExchangeScope:    {false, "scope"},
+       }
+)
+
+func parseOAuthOptions(options map[string]string, paramMap 
map[string]oAuthOption, flowName string) (map[string]string, error) {
+       params := map[string]string{}
+
+       for key, param := range paramMap {
+               if value, ok := options[key]; ok {
+                       params[key] = value
+                       delete(options, key)
+               } else if param.isRequired {
+                       return nil, fmt.Errorf("%s grant requires %s", 
flowName, key)
+               }
+       }
+
+       return params, nil
+}
+
+func exchangeToken(conf *oauth2.Config, codeOptions []oauth2.AuthCodeOption) 
(credentials.PerRPCCredentials, error) {
+       ctx := context.Background()
+       tok, err := conf.Exchange(ctx, "", codeOptions...)

Review Comment:
   `code` is specific for [authorization code flow with 
PKCE](https://oauth.net/2/grant-types/authorization-code/). It is the 
authorization code that is obtained after the user hit the URL returned from 
`conf.AuthCodeURL` and is exchanged by an access token ([3 legged 
flow](https://learn.microsoft.com/en-us/linkedin/shared/authentication/authorization-code-flow?tabs=HTTPS1#authorization-code-flow)).
   
   In the scenario of client credentials and token exchange it is not required 
([2 legged 
flows](https://learn.microsoft.com/en-us/linkedin/shared/authentication/client-credentials-flow?context=linkedin%2Fcontext&tabs=HTTPS1)).



##########
go/adbc/driver/flightsql/flightsql_database.go:
##########
@@ -160,14 +161,45 @@ 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 OR token",
                                Code: adbc.StatusInvalidArgument,
                        }
                }
                d.pass = p
                delete(cnOptions, adbc.OptionKeyPassword)
        }
 
+       if flow, ok := cnOptions[OptionKeyOauthFlow]; ok {
+               if d.hdrs.Len() > 0 {
+                       return adbc.Error{
+                               Msg:  "Authentication conflict: Use either 
Authorization header OR username/password parameter OR token",
+                               Code: adbc.StatusInvalidArgument,
+                       }
+               }
+
+               var rpcCreds credentials.PerRPCCredentials

Review Comment:
   done



-- 
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