-
Notifications
You must be signed in to change notification settings - Fork 40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Mindful auth #744
Mindful auth #744
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please modify tests for Mindful source and task if needed.
credentials_mindful["CUSTOMER_UUID"], | ||
credentials_mindful["AUTH_TOKEN"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here you are getting CUSTOMER_UUID
and AUTH_TOKEN
but docstring in the source says that The structure is user and password
. What is correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In mindful, the user is defined by an ID, in this case, CUSTOMER_UUID
, and the password is defined by, what they said AUTH TOKEN
, but this is a password, I mean, if you need to generate a token for OAuth, this not would be correct, you would have to create a token.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when I saved it to the credentials, I added the same words they decided to use for the user (mindful velux user) and the password (mindful velux password).
@@ -27,7 +28,7 @@ def __init__( | |||
"""Mindful connector which allows listing and downloading into Data Frame or specified format output. | |||
|
|||
Args: | |||
header (str): Header with credentials for calling Mindful API. | |||
auth (Tuple[str]): Authentication credentials for calling Mindful API. The structure is user and password. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docstring says that The structure is user and password
. In the task, you are getting CUSTOMER_UUID
and AUTH_TOKEN
. It doesn't look like a username and password.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But the auth accepts the function HTTPBasicAuth
which only needs the user and password.
I you think it necessary for name consistency, I could replace user
and password
in docstring by customer_uuid
and auth_token
respectively.
Summary
Mindful API has undergone a change in its structure. Since three weeks ago we are not able to access the API by adding credentials in the header. The solution given by the HCL support team is using
auth
parameter in therequest
function, and it was implemented in the present PR.On the other hand, Instead of
Bearer
authentication, a newBasic
authentication is used. The client chose this new way of authentication to avoid future problems with archived credentials after 90 days.Importance
Required by client. Very important.
Checklist
This PR:
CONTRIBUTING.md
CHANGELOG.md