Support requiring 2FA for authentication, check isEnrolledIn2Sv
#204
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rtyley
force-pushed
the
use-isEnrolledIn2Sv-to-check-2FA
branch
12 times, most recently
from
eb191bd
to
c920ab6
Compare
rtyley
changed the title
isEnrolledIn2Sv
rtyley
force-pushed
the
use-isEnrolledIn2Sv-to-check-2FA
branch
from
c920ab6
to
4611032
Compare
Using Google's Admin SDK Directory API we can check the `isEnrolledIn2Sv` field on the Directory API User entity (https://developers.google.com/admin-sdk/directory/reference/rest/v1/users) to accurately obtain the 2FA status of a user. At the Guardian, in the past, we've checked the user for membership of the `2fa_enforce@guardian.co.uk` Google Group, as a proxy for being able to directly check the 2FA status of the user. The Google Group was manually administered, so suffered from reconciliation issues, and no longer corresponds to our onboarding process. Now, if you supply a `TwoFactorAuthChecker` to the `GoogleAuthConfig`, the 2FA status of the user will be checked at the point of authentication, and rejected if `isEnrolledIn2Sv` is false - only users with 2FA will be allowed to authenticate.
rtyley
force-pushed
the
use-isEnrolledIn2Sv-to-check-2FA
branch
from
4611032
to
e6cef99
Compare
rtyley
changed the title
isEnrolledIn2SvisEnrolledIn2Sv
rtyley
commented
Nov 28, 2023
| @@ -41,6 +41,7 @@ def projectWithPlayVersion(playVersion: PlayVersion) = | |||
| "org.typelevel" %% "cats-core" % "2.9.0", | |||
| commonsCodec, | |||
| "org.scalatest" %% "scalatest" % "3.2.16" % Test, | |||
| "software.amazon.awssdk" % "ssm" % "2.21.7" % Test | |||
There was a problem hiding this comment.
This new dependency is only for the test scope, as it's only needed for the new TwoFactorAuthCheckerCLITester, only used for double-checking Google Credentials are correctly setup.
rtyley
commented
Nov 28, 2023
| ): EitherT[Future, Result, UserIdentity] = EitherT { | ||
| checker.check(userIdentity.email).map(userHas2FA => if (userHas2FA) Right(userIdentity) else { | ||
| logger.warn(s"failed-oauth-callback : user-does-not-have-2fa") | ||
| Left(redirectWithError(failureRedirectTarget, "You do not have 2FA enabled")) |
There was a problem hiding this comment.
This message will be displayed to the user if they don't have 2FA - as in this Ophan PR here:
gu-scala-steward-public-repos bot
pushed a commit
that referenced
this pull request
Nov 30, 2023
…code Tidying up code in preparation for #204 which adds support for requiring 2FA. Both `GoogleGroupChecker` (which will soon have a smaller role, as it was previously crucial for checking membership of `2fa_enforce@guardian.co.uk`) and the new `TwoFactorAuthChecker` (introduced by PR #204), require an Admin SDK `Directory` API client (but with different scopes), so `DirectoryService` is introduced to make creating one a little simpler.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
see also the Decommissioning
2fa_enforceGoogle Group docChecking users have 2FA enabled before they access internal Guardian tools is very important (good security practice, required by GDPR, etc), but in the past it's not been possible to directly check if a user has 2FA enabled - as a proxy, we've checked the user for membership of the
2fa_enforce@guardian.co.ukGoogle Group. The Google Group was manually administered, so suffered from reconciliation issues, and Patrick Simkins reports the group does not correspond well with our onboarding process.Recently, in discussion about this, @andrew-nowak pointed out that we can use Google's Admin SDK Directory API and retrieve the Directory API
Userentity, giving access to theisEnrolledIn2Svfield which accurately gives the 2FA status of a user.This PR introduces
TwoFactorAuthChecker, a class that can read theisEnrolledIn2Svfield for a given user. If you supply aTwoFactorAuthCheckerto theGoogleAuthConfig, the 2FA status of the user will be checked at authentication time, and rejected ifisEnrolledIn2Svis false - only users with 2FA will be allowed to authenticate, and it's no longer necessary, or advisable, to check the2fa_enforce@guardian.co.ukGoogle Group.Required permissions to access the
UserentityNote that this new call requires the
https://www.googleapis.com/auth/admin.directory.user.readonlyscope, and the Google Service Account used byTwoFactorAuthCheckerwill need to have the corresponding user-read permission set in order for this call to succeed. You can adapt the code inTwoFactorAuthCheckerCLITesterto double-check permissions are setup correctly.Example PR using this change
Similar changes for other Guardian libraries
@guardian/cdk- it doesn't support theisEnrolledIn2Svfield, yet