Skip to content

Commit

Permalink
Merge pull request #961 from hmrc/BDOG-3280
Browse files Browse the repository at this point in the history 
BDOG-3280: Add handling for super_admin and all_team_admins when creating new user
  • Loading branch information
@BriWak
BriWak authored Oct 31, 2024
2 parents e980461 + 0a25ffc commit c87a534
Show file tree
Hide file tree
Showing 3 changed files with 130 additions and 80 deletions.
71 changes: 40 additions & 31 deletions app/uk/gov/hmrc/cataloguefrontend/users/CreateUserController.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import uk.gov.hmrc.cataloguefrontend.auth.CatalogueAuthBuilders
import uk.gov.hmrc.cataloguefrontend.connector.UserManagementConnector
import uk.gov.hmrc.cataloguefrontend.model.TeamName
import uk.gov.hmrc.cataloguefrontend.users.view.html.{CreateUserPage, CreateUserRequestSentPage}
import uk.gov.hmrc.http.HeaderCarrier
import uk.gov.hmrc.internalauth.client.*
import uk.gov.hmrc.play.bootstrap.frontend.controller.FrontendController

Expand Down Expand Up @@ -58,8 +59,12 @@ class CreateUserController @Inject()(
auth.authenticatedAction(
continueUrl = routes.CreateUserController.createUserLanding(isServiceAccount),
retrieval = Retrieval.locations(resourceType = Some(ResourceType("catalogue-frontend")), action = Some(IAAction("CREATE_USER")))
).apply { implicit request =>
Ok(createUserPage(CreateUserForm.form, cleanseUserTeams(request.retrieval), Organisation.values.toSeq, isServiceAccount))
).async { implicit request =>
for
teams <- if request.retrieval.contains(Resource.from("catalogue-frontend", "teams/*")) then {
userManagementConnector.getAllTeams().map(_.map(_.teamName))
} else Future.successful(cleanseUserTeams(request.retrieval))
yield Ok(createUserPage(CreateUserForm.form, teams, Organisation.values.toSeq, isServiceAccount))
}

def createUser(isServiceAccount: Boolean): Action[AnyContent] =
Expand All @@ -68,44 +73,50 @@ class CreateUserController @Inject()(
retrieval = Retrieval.locations(resourceType = Some(ResourceType("catalogue-frontend")), action = Some(IAAction("CREATE_USER")))
).async { implicit request =>
(for
form <- EitherT.fromEither[Future](CreateUserForm.form.bindFromRequest().fold(
formWithErrors => {
Left(
BadRequest(
createUserPage(
form = formWithErrors,
teamNames = cleanseUserTeams(request.retrieval),
organisations = Organisation.values.toSeq,
isServiceAccount = isServiceAccount
)
)
)
},
validForm => Right(validForm)
))
_ <- EitherT.liftF(auth.authorised(Some(createUserPermission(form.team))))
res <- EitherT.right[Result](userManagementConnector.createUser(
form.copy(isServiceAccount = isServiceAccount)
))
_ = logger.info(s"user management result: $res:")
teams <- EitherT.liftF(
if request.retrieval.contains(Resource.from("catalogue-frontend", "teams/*")) then
userManagementConnector.getAllTeams().map(_.map(_.teamName))
else
Future.successful(cleanseUserTeams(request.retrieval))
)
form <- EitherT.fromEither[Future](CreateUserForm.form.bindFromRequest().fold(
formWithErrors => {
Left(
BadRequest(
createUserPage(
form = formWithErrors,
teamNames = teams,
organisations = Organisation.values.toSeq,
isServiceAccount = isServiceAccount
)
)
)
},
validForm => Right(validForm)
))
_ <- EitherT.liftF(auth.authorised(Some(createUserPermission(form.team))))
res <- EitherT.right[Result](userManagementConnector.createUser(
form.copy(isServiceAccount = isServiceAccount)
))
_ = logger.info(s"user management result: $res:")
yield Redirect(uk.gov.hmrc.cataloguefrontend.users.routes.CreateUserController.requestSent(isServiceAccount, form.givenName, form.familyName))
).merge
}

private def cleanseUserTeams(resources: Set[Resource]): Seq[TeamName] =
resources.map(_.resourceLocation.value.stripPrefix("teams/"))
.map(TeamName.apply)
.toSeq
.sorted
resources.map(_.resourceLocation.value.stripPrefix("teams/"))
.map(TeamName.apply)
.toSeq
.sorted

end CreateUserController

object CreateUserForm:
val form: Form[CreateUserRequest] =
Form(
Forms.mapping(
"givenName" -> Forms.text.verifying(CreateUserConstraints.nameConstraints("givenName")*)
.verifying(CreateUserConstraints.containsServiceConstraint),
"givenName" -> Forms.text.verifying(CreateUserConstraints.containsServiceConstraint)
.verifying(CreateUserConstraints.nameConstraints("givenName")*),
"familyName" -> Forms.text.verifying(CreateUserConstraints.nameConstraints("familyName")*),
"organisation" -> Forms.nonEmptyText,
"contactEmail" -> Forms.email.verifying(CreateUserConstraints.digitalEmailConstraint),
Expand Down Expand Up @@ -133,17 +144,15 @@ object CreateUserConstraints:
val nameLengthValidation : String => Boolean = str => str.length >= 2 && str.length <= 30
val whiteSpaceValidation : String => Boolean = str => !str.matches(".*\\s.*")
val underscoreValidation : String => Boolean = str => !str.contains("_")
val lowercaseValidation : String => Boolean = str => str.toLowerCase.equals(str)

Seq(
mkConstraint(s"constraints.${fieldName}LengthCheck" )(constraint = nameLengthValidation, error = "Should be between 2 and 30 characters long")
, mkConstraint(s"constraints.${fieldName}WhitespaceCheck")(constraint = whiteSpaceValidation, error = "Cannot contain whitespace")
, mkConstraint(s"constraints.${fieldName}UnderscoreCheck")(constraint = underscoreValidation, error = "Cannot contain underscores")
, mkConstraint(s"constraints.${fieldName}CaseCheck" )(constraint = lowercaseValidation, error = "Should only contain lowercase characters")
)

private val containsServiceValidation: String => Boolean =
!_.matches(".*\\bservice\\b.*")
!_.toLowerCase.matches(".*\\bservice\\b.*")

val containsServiceConstraint: Constraint[String] =
mkConstraint("constraints.givenNameNotAServiceCheck")(
Expand Down
135 changes: 90 additions & 45 deletions app/uk/gov/hmrc/cataloguefrontend/users/view/CreateUserPage.scala.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ <h1 class="page-heading mt-4">Create A User</h1>
There are currently no teams that you have permissions to create a user for. Ensure you are signed in to the MDTP Catalogue using your LDAP credentials (as opposed to using the guest sign-in process). If you are already signed in, then you will need to ask a team-admin to add you to your desired team on UMP.
</div>
} else {
<p class="mt-2">You will only be able to onboard users onto teams that you are a member of. If the team you wish to create a user for is not in the below list, then you will need to ask a team-admin to add you to the desired team on UMP.</p>
<p class="mt-2">You will only be able to onboard users onto teams that you are a member of. If the team you wish to create a user for is not in the list above, then you will need to ask a team-admin to add you to the desired team on UMP.</p>
}
</div>
</div>
Expand All @@ -143,60 +143,66 @@ <h1 class="page-heading mt-4">Create A User</h1>
</div>
<div class="form-group row mb-4">
<div class="col-md-6">
<label class="fw-bold">Tooling Access: </label>

<div class="row">
<div class="col-md-6 mb-2">
<div class="form-check form-check-inline">
<label class="form-check-label" for="vpnCheckbox">VPN</label>
<input class="form-check-input toolingAccessCheckbox" type="checkbox" id="vpnCheckbox" name="vpn" value=true @form.data.get("vpn").map{ _ => checked }>
</div>
<label class="fw-bold mb-2">Tooling Access: </label>

<div class="mb-2">
<div class="form-check form-check-inline">
<input class="form-check-input toolingAccessCheckbox" type="checkbox" id="vpnCheckbox" name="vpn" value=true @form.data.get("vpn").map{ _ => checked }>
<label class="form-check-label" for="vpnCheckbox"><strong>VPN</strong> <i>(Not needed for HMRC-managed STRIDE devices)</i></label>
</div>
</div>

<div class="col-md-6 mb-2">
<div class="form-check form-check-inline">
<label class="form-check-label" for="jiraCheckbox">Jira</label>
<input class="form-check-input toolingAccessCheckbox" type="checkbox" id="jiraCheckbox" name="jira" value=true @form.data.get("jira").map{ _ => checked }>
</div>
<div class="mb-2">
<div class="form-check form-check-inline">
<input class="form-check-input toolingAccessCheckbox" type="checkbox" id="jiraCheckbox" name="jira" value=true @form.data.get("jira").map{ _ => checked }>
<label class="form-check-label" for="jiraCheckbox"><strong>Jira</strong></label>
</div>
</div>

<div class="row">
<div class="col-md-6 mb-2">
<div class="form-check form-check-inline">
<label class="form-check-label" for="confluenceCheckbox">Confluence</label>
<input class="form-check-input toolingAccessCheckbox" type="checkbox" id="confluenceCheckbox" name="confluence" value=true @form.data.get("confluence").map{ _ => checked }>
</div>
<div class="mb-2">
<div class="form-check form-check-inline">
<input class="form-check-input toolingAccessCheckbox" type="checkbox" id="confluenceCheckbox" name="confluence" value=true @form.data.get("confluence").map{ _ => checked }>
<label class="form-check-label" for="confluenceCheckbox"><strong>Confluence</strong></label>
</div>
</div>

<div class="col-md-6 mb-2">
<div class="form-check form-check-inline">
<label class="form-check-label" for="environmentsCheckbox">Developer Tools (Kibana, Grafana, Jenkins)</label>
<input class="form-check-input toolingAccessCheckbox" type="checkbox" id="environmentsCheckbox" name="environments" value=true @form.data.get("environments").map{ _ => checked }>
</div>
<div class="mb-2">
<div class="form-check form-check-inline">
<input class="form-check-input toolingAccessCheckbox" type="checkbox" id="environmentsCheckbox" name="environments" value=true @form.data.get("environments").map{ _ => checked }>
<label class="form-check-label" for="environmentsCheckbox"><strong>Developer Tools</strong> <i>(Kibana, Grafana, Jenkins)</i></label>
</div>
</div>

<div class="row">
<div class="col-md-6 mb-2">
<div class="form-check form-check-inline">
<label class="form-check-label" for="googleApps">G Suite (GMail, Calendar, Hangout, Drive etc)</label>
<input class="form-check-input toolingAccessCheckbox" type="checkbox" id="googleAppsCheckbox" name="googleApps" value=true @form.data.get("googleApps").map{ _ => checked }>
</div>
<div class="mb-2">
<div class="form-check form-check-inline">
<input class="form-check-input toolingAccessCheckbox" type="checkbox" id="googleAppsCheckbox" name="googleApps" value=true @form.data.get("googleApps").map{ _ => checked }>
<label class="form-check-label" for="googleApps"><strong>Google Workspace</strong> <i>(Gmail, Calendar, Hangouts, Drive etc)</i></label>
</div>
<div class="col-md-6 mb-2">
<div class="form-check form-check-inline">
<label class="form-check-label" for="bitwardenCheckbox">BitWarden</label>
<input class="form-check-input toolingAccessCheckbox" type="checkbox" id="bitwardenCheckbox" name="bitwarden" value=true @form.data.get("bitwarden").map{ _ => checked }>
</div>
</div>

<div id="googleAppsAlert" class="border-dark-subtle border-start ms-4 mb-2 d-none" role="alert">
<div class="ms-3">
Note that once Google access has been granted, the user MUST enable two-factor authentication (2FA) in their Google account, otherwise most Google functionality will be blocked.
</div>
<div class="col-md-6 mb-2">
<button id="selectAllButton" class="btn btn-success btn-sm">Select All</button>
</div>

<div class="mb-2">
<div class="form-check form-check-inline">
<input class="form-check-input toolingAccessCheckbox" type="checkbox" id="bitwardenCheckbox" name="bitwarden" value=true @form.data.get("bitwarden").map{ _ => checked }>
<label class="form-check-label" for="bitwardenCheckbox"><strong>Bitwarden</strong> <i>(Password Manager)</i></label>
</div>
</div>

<div class="mb-2">
<button id="selectAllButton" class="btn btn-success btn-sm">Select All</button>
</div>
</div>
</div>





<div>
<button type="button" class="btn btn-success" data-bs-toggle="modal" data-bs-target="#staticBackdrop">
Create
Expand All @@ -212,9 +218,8 @@ <h5 class="modal-title" id="staticBackdropLabel">Create New User</h5>
<button type="button" class="btn-close" data-bs-dismiss="modal" aria-label="Close"></button>
</div>
<div class="modal-body">
Are you sure you want to create a new user?
<p></p>
Please check that the details are correct.
<p>Are you sure you want to create a new user?</p>
<p class="mb-0">Please check that the details are correct.</p>
</div>
<div class="modal-footer">
<button type="button" class="btn btn-secondary" data-bs-dismiss="modal">Close</button>
Expand All @@ -231,22 +236,57 @@ <h5 class="modal-title" id="staticBackdropLabel">Create New User</h5>
document.addEventListener('DOMContentLoaded', function () {
const selectAllButton = document.getElementById('selectAllButton');
const groupCheckboxes = document.querySelectorAll('.toolingAccessCheckbox');
const googleAppsAlert = document.getElementById('googleAppsAlert');
const organisationInput = document.getElementById('organisationInput');
const devToolsCheckbox = document.getElementById('environmentsCheckbox');
const googleAppsCheckbox = document.getElementById('googleAppsCheckbox');
const bitwardenCheckbox= document.getElementById('bitwardenCheckbox');

function disableAndUntick(checkbox, shouldDisable, alert = null) {
if (shouldDisable) {
checkbox.checked = false;
checkbox.disabled = true;
if (alert) alert.classList.add('d-none');
} else {
checkbox.disabled = false;
}
}

function updateToolingAccessVisibility() {
const selectedOrganisation = organisationInput.value;

disableAndUntick(devToolsCheckbox, selectedOrganisation === 'Other');
disableAndUntick(googleAppsCheckbox, selectedOrganisation === 'Other', googleAppsAlert);
disableAndUntick(bitwardenCheckbox, selectedOrganisation === 'VOA' || selectedOrganisation === 'Other');

updateButtonText();
}

function toggleAlert(checkboxId, alertId) {
const checkbox = document.getElementById(checkboxId);
const alertBox = document.getElementById(alertId);

checkbox.addEventListener('change', function() {
alertBox.classList.toggle('d-none', !checkbox.checked);
});
}

// Update button text based on checkbox states
function updateButtonText() {
const allChecked = Array.from(groupCheckboxes).every(c => c.checked);
const allChecked = Array.from(groupCheckboxes).every(c => c.checked || c.disabled);
selectAllButton.innerHTML = allChecked ? 'Deselect All' : 'Select All';
}

selectAllButton.addEventListener('click', function (event) {
event.preventDefault();

const isChecked = !Array.from(groupCheckboxes).every(c => c.checked);
const isChecked = Array.from(groupCheckboxes).some(c => !c.checked && !c.disabled);

groupCheckboxes.forEach(function (checkbox) {
checkbox.checked = isChecked;
if (!checkbox.disabled) checkbox.checked = isChecked;
});

googleAppsAlert.classList.toggle('d-none', !googleAppsCheckbox.checked);
updateButtonText();
});

Expand All @@ -264,7 +304,12 @@ <h5 class="modal-title" id="staticBackdropLabel">Create New User</h5>
});

// Initial page load
updateButtonText();
organisationInput.addEventListener('change', function() {
updateToolingAccessVisibility();
});

toggleAlert('googleAppsCheckbox', 'googleAppsAlert');
updateToolingAccessVisibility();
});
</script>
}
4 changes: 0 additions & 4 deletions test/uk/gov/hmrc/cataloguefrontend/createuser/CreateUserFormSpec.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,6 @@ class CreateUserFormSpec extends AnyWordSpec with Matchers {
isNameValid("_testuser") shouldBe false
}

"return false when name contains capital case characters" in {
isNameValid("TestUser") shouldBe false
}

"return Invalid when the name contains the word 'service'" in {
CreateUserConstraints.containsServiceConstraint("service-user") shouldBe Invalid("Should not contain 'service' - if you are trying to create a non human user, please use <a href=\"/create-service-user\"'>Create A Service Account</a> instead")
}
Expand Down

0 comments on commit c87a534

Please sign in to comment.