Access Graph: Entra ID application sync prerequisites#41650
Conversation
Current code assumes that Integration is always either AwsOidc, or an external audit storage integration
|
The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with |
justinas
added
the
no-changelog
There was a problem hiding this comment.
@mcbattirola could you do a quick sanity check on this code, as it seems like you're the original author of the audit storage integration support on the frontend?
| rpc GitlabEventsStream(stream GitlabEventsStreamRequest) returns (stream GitlabEventsStreamResponse); | ||
|
|
||
| // EntraEventsStream is a stream of commands to the Entra ID SSO importer. | ||
| rpc EntraEventsStream(stream EntraEventsStreamRequest) returns (stream EntraEventsStreamResponse); |
There was a problem hiding this comment.
It's used to send Entra events, but shouldn't be called Azure as the data comes from Azure and we will probably use it to send more Azure related things in the future?
There was a problem hiding this comment.
The issue here is that this is supposed to be called from Auth server while the other can be from discovery service tho
There was a problem hiding this comment.
Yeah, this is basically called Entra because Azure resources will likely be synced by a different loop in discovery, and thus would use a different stream.
Both processes will be using the azure-oidc integration, but the plugin will only be responsible for the Entra directory part (users, groups, applications, ...)
| // | ||
| // This data is stored here because it is not available through traditional methods (MS Graph API). | ||
| // Instead, it is fetched once during the plugin's set up using the user's credentials to connect to Azure's private API. | ||
| map<string, PluginEntraIDAppSSOSettings> app_sso_settings_cache = 1; |
There was a problem hiding this comment.
can we change this to repeated field?
IF we need to ensure uniqueness, we can do it manually
There was a problem hiding this comment.
We can, and then have a map at runtime instead. Are there known pitfalls with map fields in proto? I see that we have quite a few, but mostly for labels.
There was a problem hiding this comment.
I mean, if we ever need to support duplicates we need to change the message itself while if it's a replicated field, we do not need to change the message, we just need to change the implementation code which doesn't require any sync.
It's just to prepare for the future.
I also prefer to have the PluginEntraIDAppSSOSettings to include the name instead of the name being shared as a map key
public-teleport-github-review-bot
Bot
removed the request for review
from rudream
* 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 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 <jakub.nyckowski@goteleport.com> --------- Co-authored-by: Jakub Nyckowski <jakub.nyckowski@goteleport.com> * 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 <jakub.nyckowski@goteleport.com>
4 participants
This is a prerequisite to https://github.com/gravitational/teleport.e/pull/4180 .
Adds access graph specific settings to the Entra ID plugin. When the
access_graph_settingsfield is set (and access graph is enabled in the cluster), the Entra ID plugin will synchronize information about Entra ID Enterprise Applications to TAG.The access graph settings field has a hashmap of pre-fetched information for each SSO-enabled enterprise app. This is required because there is certain information we can not fetch at plugin's runtime using the Microsoft Graph API. See this section of the RFD for more information.
This also adds the required Access Graph protos (Entra "events stream" and
EntraApplicationstructure).Also fixed a bug where after
azure-oidcIntegration kind was introduced, this integration was treated as an external audit storage integration by the frontend, and deleting it did not work.