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

Oauth2 feature added #67

Open
wants to merge 50 commits into
base: main
Choose a base branch
from

Conversation

fepatrifork
Copy link

@fepatrifork fepatrifork commented May 8, 2024

Description

These changes add the capability to manage the Oauth2 authentication/authorization using JWT token provided by an IdP. The modification involves different levels of the system that is possible to refer as frontend for the part of the powerBI connector that is defined in Power Query (M) and backend for the part that contains the ODBC driver logic. The modification on the frontend (PowerBI connector) comprehends the management of the Authorization Code Flow "OAUTH2" using the predefined callbacks StartLogin, FinishLogin, Refresh, Logout and the configuration of the flow comes from a JSON file (template provided). This level passes the token to the backend (ODBC driver) that manages the information received to make an HTTP request with a different header. The modification includes the" ODBC Data Sources Administrator" that now allows the selection of the "OAUTH2" option for inserting a token in a created field and testing the connection. The modification of the token information has a similar treatment of the password in "BASIC" during the whole logic, with adapting due to the string size of the token. After installing the re-packaged .msi file, a system test was settled and passed using "PowerBI Desktop" and in " ODBC Data Sources Administrator" with a localhost database. The result of unit-test integration-test are made with the provided script test_runner.py modified to exclude .recipe extension that does not allow running all the tests. Test cases, documentation, user manual and DCO can be provided and expanded if there is an interest in this work and you consider it valuable.

Issues Resolved

Added Oauth2 feature

Check List

  • New functionality includes testing.
    • All tests pass, including unit test, integration test and doctest
    • Not all tests pass, including unit test, integration test and doctest
  • New functionality has been documented.
    • New functionality has javadoc added
    • New functionality has user manual doc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

README.md Outdated Show resolved Hide resolved
bi-connectors/PowerBIConnector/src/OpenSearchProject.pq Outdated Show resolved Hide resolved
bi-connectors/PowerBIConnector/src/OpenSearchProject.pq Outdated Show resolved Hide resolved
@rupal-bq
Copy link
Collaborator

Can you please take a look at failed CI?

@fepatrifork
Copy link
Author

fepatrifork commented May 21, 2024

Can you please take a look at failed CI?

I take care of the followings:

  • I have to add the DCO for all commits
  • Some weblinks are missing endpoint in old .md files (not modified from this pull request)
    - SOLUTION: modify the missing endpoint with the redirected link or refer a MD file from this repo
  • Linux environment seems having problem in CPPCheck
    - ERROR MESSAGE: "src/opensearchenlist/msdtc_enlist.cpp:1114:49: error: There is an unknown macro here somewhere.
    Configuration is required. If KEYWORD_DTC_CHECK is a macro then please configure it. [unknownMacro] strcat(dtcname,
    ";" KEYWORD_DTC_CHECK "=0");".
    - SOLUTION: pushed a solution for msdtc_enlist.cpp.

Looking forward for feedback after re-CI!

gaiksaya and others added 8 commits May 21, 2024 22:33
Signed-off-by: Sayali Gaikawad <gaiksaya@amazon.com>
Signed-off-by: fepatrifork <fepa@trifork.com>
Signed-off-by: Sayali Gaikawad <gaiksaya@amazon.com>
Signed-off-by: fepatrifork <fepa@trifork.com>
Signed-off-by: Sayali Gaikawad <gaiksaya@amazon.com>
Signed-off-by: fepatrifork <fepa@trifork.com>
Signed-off-by: fepatrifork <159044389+fepatrifork@users.noreply.github.com>
Signed-off-by: fepatrifork <fepa@trifork.com>
Signed-off-by: fepatrifork <fepa@trifork.com>
Signed-off-by: fepatrifork <fepa@trifork.com>
Signed-off-by: fepatrifork <fepa@trifork.com>
Signed-off-by: fepatrifork <159044389+fepatrifork@users.noreply.github.com>
Signed-off-by: fepatrifork <fepa@trifork.com>
fepatrifork and others added 12 commits May 21, 2024 22:33
Signed-off-by: fepatrifork <fepa@trifork.com>
Signed-off-by: fepatrifork <fepa@trifork.com>
Signed-off-by: fepatrifork <fepa@trifork.com>
Signed-off-by: fepatrifork <fepa@trifork.com>
Signed-off-by: fepatrifork <fepa@trifork.com>
Signed-off-by: fepatrifork <fepa@trifork.com>
Signed-off-by: fepatrifork <fepa@trifork.com>
Signed-off-by: fepatrifork <fepa@trifork.com>
Signed-off-by: fepatrifork <fepa@trifork.com>
Update readme OpenSearchProject.md

Signed-off-by: fepatrifork <159044389+fepatrifork@users.noreply.github.com>
Signed-off-by: fepatrifork <fepa@trifork.com>
Signed-off-by: fepatrifork <fepa@trifork.com>
Signed-off-by: fepatrifork <fepa@trifork.com>
@rupal-bq
Copy link
Collaborator

@fepatrifork Thanks for looking into CI failures. Can you please also add test cases, documentation and user manual?

@fepatrifork
Copy link
Author

fepatrifork commented May 22, 2024

@fepatrifork Thanks for looking into CI failures. Can you please also add test cases, documentation and user manual?

You're welcome. I see some other error:

  1. Link Checker now should finally ok with the last push
  2. Can you provide help with the error of cppcheck? Before was a clear error, now it seems all ok, but do not pass the test.

For the next steps:

  1. For documentation and user manual I can integrate and enrich the pre-existent .md file for PowerBI connector and for the driver. Is that ok?
  2. For the test cases will take some time.
    • SOLUTION: Added tests in test_conn.cpp in UnitTests and an IntegrationTest in ITODBCOauth folder. Integration test use
      a function that make a request to the IdP with client-credentials flow to get the token. Of course is fitted for our system, if
      you have an active webAPI with an IdP that can be reached with a test user, it will be perfect.

@rupal-bq
Copy link
Collaborator

Can you provide help with the error of cppcheck? Before was a clear error, now it seems all ok, but do not pass the test:

Sure, I'll take a look

For documentation and user manual I can integrate and enrich the pre-existent .md file for PowerBI connector and for the driver. Is that ok?

Yes

@fepatrifork
Copy link
Author

fepatrifork commented May 28, 2024

  • Some tests did not pass, because of some number of values returned are different, for example in test_query_execution.cpp the test:

all_columns_flights_count = 25 is not equal to
result -> num_fields = 27

  • I take a look over the Cppcheck error running, it seems that the only error resides inside in the vcpkg src\vcpkg_installed\..... that belongs to external packages manager I suppose. The others are only information-messages, that I hope they are not considered.

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.

None yet

3 participants