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

WPB-14306 [enterprise login] Implement common prerequisites for multiple endpoints #4364

Conversation

battermann
Copy link
Contributor

@battermann battermann commented Dec 5, 2024

https://wearezeta.atlassian.net/browse/WPB-14306

Checklist

  • Add a new entry in an appropriate subdirectory of changelog.d
  • Read and follow the PR guidelines

@echoes-hq echoes-hq bot added the echoes/initiative: scale Enterprise Readiness Initiatives label Dec 5, 2024
@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Dec 5, 2024
@battermann battermann force-pushed the WPB-14306-be-implement-common-prerequisites-for-multiple-endpoints branch from 4e8d1a4 to 43678eb Compare December 6, 2024 15:02
@battermann battermann force-pushed the WPB-14306-be-implement-common-prerequisites-for-multiple-endpoints branch from e56f371 to 6d32c35 Compare December 9, 2024 11:36
docs/src/developer/reference/config-options.md Outdated Show resolved Hide resolved
libs/wire-api/src/Wire/API/EnterpriseLogin.hs Show resolved Hide resolved
libs/wire-api/src/Wire/API/EnterpriseLogin.hs Outdated Show resolved Hide resolved
libs/wire-api/src/Wire/API/EnterpriseLogin.hs Outdated Show resolved Hide resolved
@battermann battermann marked this pull request as ready for review December 11, 2024 15:15
changelog.d/5-internal/WPB-14306 Outdated Show resolved Hide resolved
docs/src/developer/reference/config-options.md Outdated Show resolved Hide resolved
libs/wire-api/src/Wire/API/EnterpriseLogin.hs Outdated Show resolved Hide resolved
integration/test/Test/EnterpriseLogin.hs Show resolved Hide resolved
integration/test/Test/EnterpriseLogin.hs Show resolved Hide resolved
integration/test/Test/EnterpriseLogin.hs Outdated Show resolved Hide resolved
Copy link
Contributor

@fisx fisx left a comment

Choose a reason for hiding this comment

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

nice! the only thing i'm not certain about is the logic, since i haven't read the RFC too carefully. if you want, we can go through the PR and the confluence page together and see if anything is off, but if you are confident, i am, too!

integration/test/Test/EnterpriseLogin.hs Outdated Show resolved Hide resolved
integration/test/Test/EnterpriseLogin.hs Outdated Show resolved Hide resolved
integration/test/Test/EnterpriseLogin.hs Show resolved Hide resolved
integration/test/Test/EnterpriseLogin.hs Show resolved Hide resolved
{-# LANGUAGE OverloadedRecordDot #-}
{-# OPTIONS_GHC -Wno-ambiguous-fields #-}

module Wire.EnterpriseLoginSubsystem.Interpreter
Copy link
Contributor

Choose a reason for hiding this comment

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

i didn't check every details of this module against the RFC. (should i? probably...)

tools/stern/src/Stern/App.hs Outdated Show resolved Hide resolved
. userSubsystemInterpreter
. runTeamInvitationSubsystem teamInvitationSubsystemConfig
. authSubsystemInterpreter
)
)
$ runReaderT ma e

mkEnterpriseLoginSubsystemConfig :: Env -> Maybe EnterpriseLoginSubsystemConfig
mkEnterpriseLoginSubsystemConfig env = do
Copy link
Contributor

Choose a reason for hiding this comment

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

i would have made a missing audit-email field an error, unless it is explicitly stated in the RFC that we should silently stop sending emails.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem with that is that not all environments are going to have this feature. Still a valid remark!

@battermann battermann merged commit d142082 into develop Dec 13, 2024
11 checks passed
@battermann battermann deleted the WPB-14306-be-implement-common-prerequisites-for-multiple-endpoints branch December 13, 2024 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
echoes/initiative: scale Enterprise Readiness Initiatives ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants