Skip to content

[GH-240] Add support for Microsoft National Clouds#428

Open
JoKneeMo wants to merge 9 commits intomattermost:masterfrom
JoKneeMo:feature/GCCH-Support
Open

[GH-240] Add support for Microsoft National Clouds#428
JoKneeMo wants to merge 9 commits intomattermost:masterfrom
JoKneeMo:feature/GCCH-Support

Conversation

@JoKneeMo
Copy link

@JoKneeMo JoKneeMo commented Nov 3, 2024

Summary

This PR addresses #240 to add support for Microsoft National Cloud endpoints.
For my purposes I needed to connect with Microsoft GCC High.

I've added a new config item to select an Azure/Entra ID tenant type with support for Commercial, GCC/GCCH, US Gov (DoD), and China endpoints.

To address the login URLs, I overrode the oauth2/microsoft package to accept the tenant type config entry and return the proper endpoint.

Addressing the MS Graph API endpoints was a little more challenging.
The msgraph module was very out of date and did not support the SetURL function added 4 years ago.
That was added to the module to override the default url of graph.microsoft.com.
The MSGraph endpoint is controlled using the same new config item and returns the proper graph url.
The tokens used by graph are not cross cloud compatible.

I'm unsure of how to write the unit test mockups in this repo.

Additional Information

This logic could also be applied to the mattermost-plugin-msteams plugin and would address mattermost/mattermost-plugin-msteams#514. However, that issue is not marked as Help Wanted for community contribution. I'd be happy to open a PR for that one too otherwise I will wait for Mattermost Support to resolve it.

Ticket Link

Fixes #240

UI Screenshot

image

@JoKneeMo JoKneeMo requested a review from wiggin77 as a code owner November 3, 2024 04:57
@mattermost-build
Copy link
Contributor

Hello @JoKneeMo,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

@JoKneeMo JoKneeMo changed the title Add support for Microsoft National Clouds [GH-240] Add support for Microsoft National Clouds Nov 3, 2024
@wiggin77 wiggin77 added 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester labels Nov 6, 2024
@wiggin77
Copy link
Member

wiggin77 commented Nov 6, 2024

@spirosoik can you assign someone from your team (or yourself) to look at this from an operations perspective?

@raghavaggarwal2308
Copy link

@JoKneeMo Can you please fix the failing CI and conflicts?

@mattermost-build
Copy link
Contributor

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

@JoKneeMo
Copy link
Author

I've been busy but plan to correct the issues and update the PR this week.

@raghavaggarwal2308
Copy link

@JoKneeMo Any updates on the above?

@wiggin77
Copy link
Member

@spirosoik can you assign someone from your team (or yourself) to look at this from an operations perspective?

@spirosoik bump

Copy link
Author

@JoKneeMo JoKneeMo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Kshitij-Katiyar Thanks for the assist in getting the lint issues resolved. I've been too busy to get back to this.

@wiggin77 @spirosoik All conflicts look to be resolved now.

@angeloskyratzakos angeloskyratzakos self-requested a review February 7, 2025 19:04
Copy link

@nickmisasi nickmisasi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small nit - otherwise. LGTM. Thanks @JoKneeMo!

@mattermost-build
Copy link
Contributor

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

@Kshitij-Katiyar
Copy link
Contributor

@wiggin77 We have resolved the conflicts on this PR.

Copy link
Member

@wiggin77 wiggin77 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 Thanks @JoKneeMo

@Kshitij-Katiyar
Copy link
Contributor

@AayushChaudhary0001 Please test this PR

@AayushChaudhary0001
Copy link

@JoKneeMo @wiggin77 Do we need to change anything in our Azure account to test this PR?

@JoKneeMo
Copy link
Author

@JoKneeMo @wiggin77 Do we need to change anything in our Azure account to test this PR?

If your Azure account is commercial, there are no changes needed.

If you're looking to validate the USGov, GCC, GCCH, and China endpoints, you would need to create a new Entra tenant (domain) in those products.
For USGov, GCC, and GCCH, you can create a new trial here: https://usgovintake.embark.microsoft.com/
I'm not sure how to create one for China.

I've validated GCCH and USGov with these changes.
Commercial and GCC use the same endpoints.

@mattermost-build
Copy link
Contributor

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

@JoKneeMo
Copy link
Author

@AayushChaudhary0001 Did you need any further details?

@arush-vashishtha
Copy link

Hey @JoKneeMo, I have filled out the intake form present here, and this is the window it redirected me to,
image

Also, we do not access to add custom domains
image

@JoKneeMo
Copy link
Author

@arush-vashishtha To get a trial from that link for USGov or GCCH, you'll need a "sponsor" from a US Gov agency.
I can't really help you there.
You fill out the form with the actual intended purpose they may approve the trial anyway?
Perhaps @Nick42- from #240 would be willing to test?

@arush-vashishtha
Copy link

Hey @JoKneeMo, we discussed with @wiggin77, and he has asked to put this on hold for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester Lifecycle/1:stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Change Azure URL?

9 participants