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
feat(hc): Use hybrid cloud services in AuthIdentityHandler #43035
Changes from 2 commits
e564e68
09b79a9
279e596
21fd725
64cc26e
3c3e553
821fdf7
d55f736
ae72361
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
from __future__ import annotations | ||
|
||
import abc | ||
from dataclasses import dataclass | ||
from typing import Any, Mapping | ||
|
||
from sentry.services.hybrid_cloud import InterfaceWithLifecycle, silo_mode_delegation, stubbed | ||
from sentry.services.hybrid_cloud.auth import ApiAuthIdentity, ApiAuthProvider | ||
from sentry.services.hybrid_cloud.organization import ApiOrganization, ApiOrganizationMember | ||
from sentry.services.hybrid_cloud.user import APIUser | ||
from sentry.silo import SiloMode | ||
|
||
|
||
@dataclass | ||
class AuditLogMetadata: | ||
organization: ApiOrganization | ||
event: int | ||
actor_label: str | None = None | ||
actor: APIUser | None = None | ||
ip_address: str | None = None | ||
target_object: int | None = None | ||
target_user: APIUser | None = None | ||
|
||
|
||
class AuditLogService(InterfaceWithLifecycle): | ||
@abc.abstractmethod | ||
def write_audit_log( | ||
self, | ||
*, | ||
metadata: AuditLogMetadata, | ||
data: Mapping[str, Any], | ||
) -> None: | ||
raise NotImplementedError | ||
|
||
@abc.abstractmethod | ||
def log_organization_membership( | ||
self, | ||
*, | ||
metadata: AuditLogMetadata, | ||
organization_member: ApiOrganizationMember, | ||
) -> None: | ||
raise NotImplementedError | ||
Comment on lines
+36
to
+42
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you thinking that there will be a method for each audit log that is recorded? We have quite a few audit log event types which will make this interface wide. We should be able to extend the existing AuditLogEvent plumbing code to be aware of required data attributes which would let us have an interface like audit_log_service.log(AuditLogMetaData(
organization=self.organization,
actor=self.user,
ip_address=self.request.META["REMOTE_ADDR"],
target_object=auth_identity.id,
event=audit_log.get_event_id("SSO_IDENTITY_LINK"),
auth_identity=auth_identity
) With required parameters embedded in to the |
||
|
||
@abc.abstractmethod | ||
def log_auth_provider( | ||
self, | ||
*, | ||
metadata: AuditLogMetadata, | ||
provider: ApiAuthProvider, | ||
) -> None: | ||
raise NotImplementedError | ||
|
||
@abc.abstractmethod | ||
def log_auth_identity( | ||
self, | ||
*, | ||
metadata: AuditLogMetadata, | ||
auth_identity: ApiAuthIdentity, | ||
) -> None: | ||
raise NotImplementedError | ||
|
||
def close(self) -> None: | ||
pass | ||
|
||
|
||
def impl_with_db() -> AuditLogService: | ||
from sentry.services.hybrid_cloud.audit.impl import DatabaseBackedAuditLogService | ||
|
||
return DatabaseBackedAuditLogService() | ||
|
||
|
||
audit_log_service: AuditLogService = silo_mode_delegation( | ||
{ | ||
SiloMode.MONOLITH: impl_with_db, | ||
SiloMode.CONTROL: impl_with_db, | ||
SiloMode.REGION: stubbed(impl_with_db, SiloMode.CONTROL), | ||
} | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
You could also make this conditional to tests by
Which means you can safely continue to leave it.