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

Entra ID integration: integration script updates and web onboarding prerequisites #42172

Merged
merged 21 commits into from
May 31, 2024

Conversation

justinas
Copy link
Contributor

@justinas justinas commented May 30, 2024

This (together with the Enterprise counterpart https://github.com/gravitational/teleport.e/pull/4273) is the last large changeset for the initial version of Entra ID integration.

This PR:

  • Updates Prehog protos with values for Entra ID (as introduced in https://github.com/gravitational/cloud/pull/9111)
  • Fixes a couple of bugs in the integration script discovered while testing end-to-end.
    • Onboarding script now calls az login to make sure we have the necessary token to make calls to the "private" Azure API during onboarding.
  • Adds prerequisites for Enterprise web changes

changelog: Added support for Microsoft Entra ID directory synchronization (Teleport Enterprise only, preview)

@justinas justinas changed the title Entra ID integration: integration script updates and Entra ID integration: integration script updates and web onboarding prerequisites May 30, 2024
justinas added 5 commits May 30, 2024 18:34
Lets user know that `az login` has completed
and `teleport` is continuing its work.
CTA is 1-to-1 with prehog, but IntegrationEnrollKind is not.
This parameter is no longer accepted by the endpoint
@@ -26,6 +26,8 @@ const UPGRADE_TEAM_URL = 'https://goteleport.com/r/upgrade-team';
const UPGRADE_COMMUNITY_URL = 'https://goteleport.com/r/upgrade-community';
// UPGRADE_IGS_URL is enterprise upgrading to enterprise with Identity Governance & Security
const UPGRADE_IGS_URL = 'https://goteleport.com/r/upgrade-igs';
// UPGRADE_POLICY_URL is enterprise upgrading to enterprise with Policy
export const UPGRADE_POLICY_URL = 'https://goteleport.com/r/upgrade-policy';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

i'm sorry, i think i led you astray with how the destination link should be crafted (before policy, it didn't matter so much), do you mind changing both igs and policy destination? https://github.com/gravitational/next/pull/2505#discussion_r1621543575

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We only need to change the redirect destination in next, the URLs in the frontend can stay, right?

@justinas justinas marked this pull request as ready for review May 30, 2024 16:56
@github-actions github-actions bot requested review from avatus and rudream May 30, 2024 16:57
Copy link

The PR changelog entry failed validation: The changelog entry must start with a letter.

@justinas justinas requested review from jakule and tigrato May 30, 2024 16:57
@justinas justinas requested review from kimlisa and removed request for avatus and rudream May 30, 2024 16:57
Copy link

The PR changelog entry failed validation: The changelog entry must start with a letter.

e Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this intentional? From what I see this points to your dev branch and not master

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not intentional, will revert 👍

Comment on lines +48 to +50
if (!url) {
url = UPGRADE_COMMUNITY_URL;
if (isEnterprise) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit:

Suggested change
if (!url) {
url = UPGRADE_COMMUNITY_URL;
if (isEnterprise) {
if (!url && isEnterprise) {
url = UPGRADE_COMMUNITY_URL;

Copy link
Contributor

Choose a reason for hiding this comment

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

not sure that means the same thing 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, if url is defined we want to use it, no matter if enterprise or not.

@@ -26,6 +26,8 @@ const UPGRADE_TEAM_URL = 'https://goteleport.com/r/upgrade-team';
const UPGRADE_COMMUNITY_URL = 'https://goteleport.com/r/upgrade-community';
// UPGRADE_IGS_URL is enterprise upgrading to enterprise with Identity Governance & Security
const UPGRADE_IGS_URL = 'https://goteleport.com/r/upgrade-igs';
// UPGRADE_POLICY_URL is enterprise upgrading to enterprise with Policy
export const UPGRADE_POLICY_URL = 'https://goteleport.com/r/upgrade-policy';
Copy link
Contributor

Choose a reason for hiding this comment

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

i'm sorry, i think i led you astray with how the destination link should be crafted (before policy, it didn't matter so much), do you mind changing both igs and policy destination? https://github.com/gravitational/next/pull/2505#discussion_r1621543575

Comment on lines +48 to +50
if (!url) {
url = UPGRADE_COMMUNITY_URL;
if (isEnterprise) {
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure that means the same thing 🤔

@public-teleport-github-review-bot public-teleport-github-review-bot bot removed the request for review from tigrato May 31, 2024 13:53
@justinas justinas added this pull request to the merge queue May 31, 2024
Merged via the queue into master with commit 5eb9aa9 May 31, 2024
43 checks passed
@justinas justinas deleted the justinas/entra-plugin-onboard-web branch May 31, 2024 14:12
@public-teleport-github-review-bot

@justinas See the table below for backport results.

Branch Result
branch/v16 Failed

justinas added a commit that referenced this pull request Jun 3, 2024
…rerequisites (#42172)

* Remove integration name validation from web script

Not used by the script. It is validated by the "plugins/validate"
endpoint.

* Add required frontend constants for Entra ID

* Support Azure/Entra integrations in the list

* Add IsPolicyEnabled to web config

* Allow custom URL for ButtonLockedFeature

* Add CTA_ENTRA_ID event type

* Expose TAGInfoCache for use in e

* Add LackingIgs option

* Add Entra ID icon

* Add Entra ID plugin to storybook

* Bump e for dev build

* Return underlying error in getPrivateAPIToken

* Find default Azure subscription instead of the first one

* Require user to re-login when provisioning Azure OIDC

* Update prehog protos with Entra ID values

From https://github.com/gravitational/cloud/pull/9111

* Suppress verbose warnings / information from az

* Add an additional message after successful auth

Lets user know that `az login` has completed
and `teleport` is continuing its work.

* Move EntraId constant to the bottom

* Revert unintended changes to usageevents

CTA is 1-to-1 with prehog, but IntegrationEnrollKind is not.

* Remove integrationName validation asserts from test

This parameter is no longer accepted by the endpoint

* Revert "Bump e for dev build"

This reverts commit fc747a0.
github-merge-queue bot pushed a commit that referenced this pull request Jun 4, 2024
…rerequisites (#42172) (#42294)

* Remove integration name validation from web script

Not used by the script. It is validated by the "plugins/validate"
endpoint.

* Add required frontend constants for Entra ID

* Support Azure/Entra integrations in the list

* Add IsPolicyEnabled to web config

* Allow custom URL for ButtonLockedFeature

* Add CTA_ENTRA_ID event type

* Expose TAGInfoCache for use in e

* Add LackingIgs option

* Add Entra ID icon

* Add Entra ID plugin to storybook

* Bump e for dev build

* Return underlying error in getPrivateAPIToken

* Find default Azure subscription instead of the first one

* Require user to re-login when provisioning Azure OIDC

* Update prehog protos with Entra ID values

From https://github.com/gravitational/cloud/pull/9111

* Suppress verbose warnings / information from az

* Add an additional message after successful auth

Lets user know that `az login` has completed
and `teleport` is continuing its work.

* Move EntraId constant to the bottom

* Revert unintended changes to usageevents

CTA is 1-to-1 with prehog, but IntegrationEnrollKind is not.

* Remove integrationName validation asserts from test

This parameter is no longer accepted by the endpoint

* Revert "Bump e for dev build"

This reverts commit fc747a0.
justinas added a commit that referenced this pull request Jun 6, 2024
…rerequisites (#42172)

* Remove integration name validation from web script

Not used by the script. It is validated by the "plugins/validate"
endpoint.

* Add required frontend constants for Entra ID

* Support Azure/Entra integrations in the list

* Add IsPolicyEnabled to web config

* Allow custom URL for ButtonLockedFeature

* Add CTA_ENTRA_ID event type

* Expose TAGInfoCache for use in e

* Add LackingIgs option

* Add Entra ID icon

* Add Entra ID plugin to storybook

* Bump e for dev build

* Return underlying error in getPrivateAPIToken

* Find default Azure subscription instead of the first one

* Require user to re-login when provisioning Azure OIDC

* Update prehog protos with Entra ID values

From https://github.com/gravitational/cloud/pull/9111

* Suppress verbose warnings / information from az

* Add an additional message after successful auth

Lets user know that `az login` has completed
and `teleport` is continuing its work.

* Move EntraId constant to the bottom

* Revert unintended changes to usageevents

CTA is 1-to-1 with prehog, but IntegrationEnrollKind is not.

* Remove integrationName validation asserts from test

This parameter is no longer accepted by the endpoint

* Revert "Bump e for dev build"

This reverts commit fc747a0.
github-merge-queue bot pushed a commit that referenced this pull request Jun 7, 2024
* Entra ID reconciler: directory reconciler prerequisites (#40778)

* Add Entra ID resource origin

* Ignore ID and Revision from `header` in cmp

* Add e_imports for MS Graph SDK

* Entra ID integration: add proto definitions (#40997)

* Entra ID integration boilerplate (#40998)

* Add e imports for MS Graph SDK

* Add ability to sign Entra ID OIDC JWTs, rework KID handling

- Synthesize Key IDs for our JWT keys. For backwards compatibility, also
  include the same keys with an empty `kid` in JWKS.
- Sign AWS OIDC tokens with a `kid=""` header claim,
  rather than omitting the `kid` claim altogether.
  See comment for details.

* Add validation for Entra ID plugin

* Fix typo in assertion function name

* Update the OIDC JWKS test to expect the same key twice

* Add Entra ID plugin type constant

* go mod tidy

* Fix expected JWKS size in integration test

* Add basic tests for KeyID

* Move Azure auth settings from Plugin to Integration

* Address review comments

* Add a unit test to ensure KeyID compatibility

* Add license header to token_generator.go

* Rename validation function per new conventions

* Access Graph: sync AWS identity providers  (#41368)

* Add AWSSAMLProviderV1 to access graph proto

* Access Graph: sync AWS SAML Providers

* Parse SAML entity descriptor before sending to TAG

* Add protos for AWS OIDC providers

* Fetch AWS OIDC providers

* Fetch signing certificates for AWS SAML providers

* Deflake identity provider fetch test

The concrete implementation of IAM mock uses a map,
resulting in non-deterministic iteration order.
Sort the results before comparing to alleviate.

* Update lib/srv/discovery/fetchers/aws-sync/iam_test.go

Co-authored-by: Jakub Nyckowski <[email protected]>

---------

Co-authored-by: Jakub Nyckowski <[email protected]>

* Access Graph: Entra ID application sync prerequisites (#41650)

* Add access graph settings to Entra ID plugin

* Move Entra ID labels to OSS

* Add Entra resources and RPC to Access Graph proto

* Add azure-oidc integration to web.

Current code assumes that Integration is always either AwsOidc,
or an external audit storage integration

* Change app sso cache to a repeated field

* Entra ID integration: add onboarding script (#41811)

* Add Entra ID integration onboarding script

* Adapt after proto update

* Validate names in azure script handler, add test

* Add license headers

* Update Entra plugin test with SSO connector field

* Fix lint

* Remove leftover panics

* Adjust success message

* Downgrade log message level

* Expect exactly 1 SP for MS Graph, improve errors

* Properly extract hostname for enterprise app name

* Comment on assuming the first subscription

* Address review nits

* Factor out sso info fetch into a function

* fixup refactor

* Add retry logic to app role assignment

* Make godoc conventional

* Entra ID integration: integration script updates and web onboarding prerequisites (#42172)

* Remove integration name validation from web script

Not used by the script. It is validated by the "plugins/validate"
endpoint.

* Add required frontend constants for Entra ID

* Support Azure/Entra integrations in the list

* Add IsPolicyEnabled to web config

* Allow custom URL for ButtonLockedFeature

* Add CTA_ENTRA_ID event type

* Expose TAGInfoCache for use in e

* Add LackingIgs option

* Add Entra ID icon

* Add Entra ID plugin to storybook

* Bump e for dev build

* Return underlying error in getPrivateAPIToken

* Find default Azure subscription instead of the first one

* Require user to re-login when provisioning Azure OIDC

* Update prehog protos with Entra ID values

From https://github.com/gravitational/cloud/pull/9111

* Suppress verbose warnings / information from az

* Add an additional message after successful auth

Lets user know that `az login` has completed
and `teleport` is continuing its work.

* Move EntraId constant to the bottom

* Revert unintended changes to usageevents

CTA is 1-to-1 with prehog, but IntegrationEnrollKind is not.

* Remove integrationName validation asserts from test

This parameter is no longer accepted by the endpoint

* Revert "Bump e for dev build"

This reverts commit fc747a0.

* `go mod tidy` secondary modules

---------

Co-authored-by: Jakub Nyckowski <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants