Add bearer authentication #55
Conversation
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
danielgattiluxor
force-pushed
the
add-bearer-authorization
branch
from
10a2b71 to
88b8b7f
Compare
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
| @@ -130,6 +130,8 @@ const ( | |||
| trinoAddedPrepareHeader = trinoHeaderPrefix + `Added-Prepare` | |||
| trinoDeallocatedPrepareHeader = trinoHeaderPrefix + `Deallocated-Prepare` | |||
|
|
|||
| authorization = `Authorization` | |||
There was a problem hiding this comment.
Isn't this supposed to be Authentication?
There was a problem hiding this comment.
No, this would be Authorization to match "Authorization: Bearer {token}"
| @@ -71,6 +71,10 @@ This driver supports Kerberos authentication by setting up the Kerberos fields i | |||
|
|
|||
| Please refer to the [Coordinator Kerberos Authentication](https://trino.io/docs/current/security/server.html) for server-side configuration. | |||
|
|
|||
| #### Bearer authentication | |||
|
|
|||
| This driver supports Bearer authentication (also called token authentication) by setting it in the [Config](https://godoc.org/github.com/trinodb/trino-go-client/trino#Config) struct. | |||
There was a problem hiding this comment.
This needs an example, especially if users are supposed to prefix any tokens with Bearer.
| @@ -227,6 +230,7 @@ func (c *Config) FormatDSN() (string, error) { | |||
| "session_properties": strings.Join(sessionkv, ","), | |||
| "extra_credentials": strings.Join(credkv, ","), | |||
| "custom_client": c.CustomClientName, | |||
| "bearer_auth": c.BearerAuth, | |||
There was a problem hiding this comment.
This needs a test, ideally an integration one, where you would configure the Trino server to accept this kind of authentication.
There was a problem hiding this comment.
I would like this feature , if its not too much to ask, can I continue on this PR?
There was a problem hiding this comment.
Sure, you can copy this branch into your fork and open up a second PR.
| @@ -332,6 +336,7 @@ func newConn(dsn string) (*Conn, error) { | |||
| trinoSchemaHeader: query.Get("schema"), | |||
| trinoSessionHeader: query.Get("session_properties"), | |||
| trinoExtraCredentialHeader: query.Get("extra_credentials"), | |||
| authorization: query.Get("bearer_auth"), | |||
There was a problem hiding this comment.
We need to change the code a bit in the line#342 as
if v != "" {
if k == "Authorization" {
c.httpHeaders.Add(k, fmt.Sprintf("Bearer %s", v))
}else {
c.httpHeaders.Add(k, v)
}
|
Superseded by #113 |
Add bearer authentication