makes credentials expiration UTC/localtime aware

[ivica-k]

Connecting to a Redshift cluster with Okta as the IdP was causing the login_url to be opened twice. The original issue is the fact that STS credentials expiration datetime was set in UTC and checked against local time (which is UTC+2 in my case). Local time was always an hour ahead (UTC+2 - STS session length of 1 hour) in my case and credentials were considered expired as soon as they were created.

Additionally fixes test collection by changing the setup.cfg - this could potentially be a different PR.

Logs look like this

2021-07-15 13:55:08,636 - redshift_connector.credentials_holder - DEBUG - Credentials will expire at 2021-07-15 12:55:08+00:00 (UTC), local time is 2021-07-15 13:55:08.636606+02:00.
Difference between local time and UTC time is 2 hours.

Description

Motivation and Context

Local time was always an hour ahead (UTC+2 - STS session length of 1 hour) in my case and credentials were considered expired as soon as they were created. Fixes #40

Testing

Executed the following script in the CEST (Europe/Amsterdam) to verify that login_url was opened only once

import redshift_connector


conn = redshift_connector.connect(
    iam=True,
    ssl=True,
    host="REDSHIFT_ENDPOINT",
    port=5439,
    database="DB_NAME",
    db_user="IDP_USERNAME",
    region="AWS_REGION",
    cluster_identifier="CLUSTER_NAME",
    login_url="IDP_LOGIN_URL",
    credentials_provider="BrowserSamlCredentialsProvider",
    user="",
    password=""
)

cursor: redshift_connector.Cursor = conn.cursor()
cursor.execute("select current_user")

result = cursor.fetchone()
print(result)

It would be great to have unit tests for this change but I am unsure how to

Screenshots (if appropriate)

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)

Checklist

• Local run of ./build.sh succeeds
• Code changes have been run against the repository's pre-commit hooks
• Commit messages follow Conventional Commit Specification
• I have read the README document
• I have added tests to cover my changes
• I have run all unit tests using pytest test/unit and they are passing.

• By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[Brooke-white]

Thank you @ivica-k ! We will run this PR through our internal test suite and follow up with any feedback next week :)

[ivica-k]

I'm not entirely satisfied with the tests I wrote.

[Brooke-white]

From the datetime docs, here I think we could use datetime.now(timezone.utc) instead of manually applying the utc offset.

[ivica-k]

A much better solution! Implemented in 922d7b3

[Brooke-white]

The Expiration we receive from boto3 is a datetime with UTC timezone. I think we could replace datetime.datetime.now() with datetime.now(timezone.utc) to achieve that. This would allow us to directly use expiration when creating the credentialsdict

[Brooke-white]

Could we use add a minute or so onto this test case (i.e. datetime.datetime.now() + datetime.timedelta(minutes=1))? It's expired by the time the is_expired() check occurs on my machine.

[ivica-k]

Thank you, I understand your point. Addressed in 922d7b3

[Brooke-white]

LGTM -- thank you!

[ivica-k]

LGTM -- thank you!

Woot! That was super quick, thank you!