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

Add login with Azure AD app certificate and Sites.Selected #48

Conversation

rikroe
Copy link

@rikroe rikroe commented Aug 4, 2023

This PR adds an additional preset and login mechanism: Azure AD app registrations with certificate authentication.

If there are any change requests in terms of wording for this PR or suggestions how to better deal with the private key in the parameters (especially regarding spaces/newlines), happy to incorporate them. The client_secret can unfortunately not be used.

I've added the msal library to do the authentication, but it could be done manually using PyJWT[crypto] and cryptography.

Reason

Microsoft is by default deactivating the ability to add new Site App registrations or change those permissions by mid of September (see MC660075).
While it can be turned on again, our Sharepoint Admins won't do this.

To access specific sites only, one can create an App Registration in Azure AD and grant the Sharepoint Sites.Selected permission. While this was orignally only available to Graph APIs, it is now possible to use these permissions with the original Sharepoint REST API (as used by this plugin) as well.
More information and how to grant permission can be found in the announcement blog post and this detailled blog post.

@alexbourret alexbourret changed the base branch from master to feature/sc-147685-review-external-pr-site-app-registration October 17, 2023 12:57
@pkbullock
Copy link

pkbullock commented Jun 3, 2024

Hi @alexbourret - I have interest in this option too, is there any movement on this feature? Currently, the plugin isnt using the most secure methods of connecting to the product.

Another question, if this change was to be merged, how is this released to the product, would a new release be needed, or can we just perform an update? (Sorry not overly familiar with the plugin part of the product)

@rikroe
Copy link
Author

rikroe commented Jun 3, 2024

Another question, if this change was to be merged, how is this released to the product, would a new release be needed, or can we just perform an update? (Sorry not overly familiar with the plugin part of the product)

You just need to update the plugin, not Dataiku itself.

We are running with this we created this PR.

@alexbourret please let me know if I should rebase this PR (and onto which branch) or if any other changes are required.

@alexbourret
Copy link
Collaborator

Hi @pkbullock - We just got the new sharepoint QA instance to test this feature, so it is next on the list
@rikroe - Thank you for the PR ! No need to rebase, I can do it since we have to add a few things (tests, support for keys containing "encrypted" in the header...)

@pkbullock
Copy link

Thank you @alexbourret - do you have a ball park timeline for this?

@alexbourret
Copy link
Collaborator

@pkbullock - the feature is now released in v1.1.3.
@rikroe - Thank you again for your contribution ! I'm closing this PR but your code is now included and I've added your name to the contributors file.

@alexbourret
Copy link
Collaborator

Feature merged with PR #50

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