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?
| @@ -82,6 +83,9 @@ service AccessGraphService { | |||
|
|
|||
| // GitlabEventsStream is a stream of commands to the Gitlab importer. | |||
| 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
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.