Skip to content
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

Merged
merged 2 commits into from
Oct 2, 2023
Merged

Mindful auth #744

merged 2 commits into from
Oct 2, 2023

Conversation

Diego-H-S
Copy link
Contributor

@Diego-H-S Diego-H-S commented Sep 12, 2023

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 the request function, and it was implemented in the present PR.

On the other hand, Instead of Bearer authentication, a new Basic 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:

  • follows the guidelines laid out in CONTRIBUTING.md
  • links relevant issue(s)
  • adds/updates tests (if appropriate)
  • adds/updates docstrings (if appropriate)
  • adds an entry in CHANGELOG.md

@Diego-H-S Diego-H-S mentioned this pull request Sep 13, 2023
Copy link
Contributor

@Rafalz13 Rafalz13 left a 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.

Comment on lines +121 to +122
credentials_mindful["CUSTOMER_UUID"],
credentials_mindful["AUTH_TOKEN"],
Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Contributor Author

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.
Copy link
Contributor

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.

Copy link
Contributor Author

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.

@Rafalz13 Rafalz13 merged commit 37909e0 into dyvenia:dev Oct 2, 2023
3 checks passed
@Diego-H-S Diego-H-S deleted the mindful_auth branch June 7, 2024 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants