Skip to content

Commit

Permalink
gh-4594 Make account create/update create/update stroom user
Browse files Browse the repository at this point in the history
  • Loading branch information
at055612 committed Nov 8, 2024
1 parent f78beeb commit a86ddfc
Show file tree
Hide file tree
Showing 9 changed files with 145 additions and 69 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import stroom.security.shared.UserResource;
import stroom.svg.shared.SvgImage;
import stroom.util.shared.GwtNullSafe;
import stroom.util.shared.UserDesc;
import stroom.widget.popup.client.event.HidePopupRequestEvent;
import stroom.widget.popup.client.event.ShowPopupEvent;
import stroom.widget.popup.client.presenter.PopupSize;
Expand Down Expand Up @@ -162,34 +161,11 @@ private void createAccount(final HidePopupRequestEvent e) {
.create(ACCOUNT_RESOURCE)
.method(res -> res.create(request))
.onSuccess(id -> {

// If we successfully created the account then also automatically create a permissions
// user
createUser(request.getUserId(), e);

})
.onFailure(throwable ->
AlertEvent.fireError(this, "Error creating account: "
+ throwable.getMessage(), e::reset))
.taskMonitorFactory(this)
.exec();
}

private void createUser(final String userId,
final HidePopupRequestEvent e) {
final UserDesc userDesc = new UserDesc(
userId,
userId,
userId);
restFactory
.create(USER_RESOURCE)
.method(res -> res.createUser(userDesc))
.onSuccess(user -> {
onChangeHandler.run();
e.hide();
})
.onFailure(throwable ->
AlertEvent.fireError(this, "Error creating user: "
AlertEvent.fireError(this, "Error creating account: "
+ throwable.getMessage(), e::reset))
.taskMonitorFactory(this)
.exec();
Expand Down Expand Up @@ -222,6 +198,9 @@ private void updateAccount(final HidePopupRequestEvent e) {
}


// --------------------------------------------------------------------------------


public interface EditAccountView extends View, Focus, HasUiHandlers<EditAccountUiHandlers> {

void setUserId(String userId);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package stroom.security.api;

import stroom.util.shared.UserDesc;

import jakarta.servlet.http.HttpServletRequest;

import java.util.Map;
Expand Down Expand Up @@ -58,4 +60,14 @@ public interface UserIdentityFactory {
Map<String, String> getAuthHeaders(final String jwt);

void refresh(final UserIdentity userIdentity);

/**
* Allows an identity provider to manually add a userIdentity, e.g. so it is available
* in stroom for permissions to be granted to it prior to the user logging in.
* If the userIdentity is present, its displayName and fullName will be updated using
* the values from userIdentity.
*/
default void ensureUserIdentity(final UserDesc userDesc) {
throw new UnsupportedOperationException("Ensuring user identities manually is not supported");
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package stroom.security.identity.account;

import stroom.security.api.SecurityContext;
import stroom.security.api.UserIdentityFactory;
import stroom.security.identity.authenticate.PasswordValidator;
import stroom.security.identity.config.IdentityConfig;
import stroom.security.identity.shared.Account;
Expand All @@ -10,26 +11,32 @@
import stroom.security.identity.shared.UpdateAccountRequest;
import stroom.security.shared.AppPermission;
import stroom.util.shared.PermissionException;
import stroom.util.shared.UserDesc;

import com.google.common.base.Strings;
import jakarta.inject.Inject;
import org.jetbrains.annotations.NotNull;

import java.util.Objects;
import java.util.Optional;

public class AccountServiceImpl implements AccountService {

private final AccountDao accountDao;
private final SecurityContext securityContext;
private final IdentityConfig config;
private final UserIdentityFactory userIdentityFactory;


@Inject
AccountServiceImpl(final AccountDao accountDao,
final SecurityContext securityContext,
final IdentityConfig config) {
final IdentityConfig config,
final UserIdentityFactory userIdentityFactory) {
this.accountDao = accountDao;
this.securityContext = securityContext;
this.config = config;
this.userIdentityFactory = userIdentityFactory;
}

// @Override
Expand Down Expand Up @@ -131,17 +138,24 @@ public AccountResultPage search(final FindAccountRequest request) {
public Account create(final CreateAccountRequest request) {
checkPermission();
validateCreateRequest(request);
final Account account = buildAccountObject(request);
final Account persistedAccount = accountDao.create(account, request.getPassword());

// Validate
final String userId = securityContext.getUserIdentityForAudit();
// Create a corresponding stroom user for the account
userIdentityFactory.ensureUserIdentity(createUserIdentity(account));

final long now = System.currentTimeMillis();
return persistedAccount;
}

@NotNull
private Account buildAccountObject(final CreateAccountRequest request) {
final long now = System.currentTimeMillis();
final String userIdForAudit = securityContext.getUserIdentityForAudit();
final Account account = new Account();
account.setCreateTimeMs(now);
account.setCreateUser(userId);
account.setCreateUser(userIdForAudit);
account.setUpdateTimeMs(now);
account.setUpdateUser(userId);
account.setUpdateUser(userIdForAudit);
account.setFirstName(request.getFirstName());
account.setLastName(request.getLastName());
account.setUserId(request.getUserId());
Expand All @@ -152,8 +166,17 @@ public Account create(final CreateAccountRequest request) {
account.setLoginCount(0);
// Set enabled by default.
account.setEnabled(true);
return account;
}

return accountDao.create(account, request.getPassword());
private UserDesc createUserIdentity(final Account account) {
if (account == null) {
return null;
} else {
return UserDesc.builder(account.getUserId())
.fullName(account.getFullName())
.build();
}
}

@Override
Expand Down Expand Up @@ -184,10 +207,8 @@ public void update(final UpdateAccountRequest request, final int accountId) {
checkPermission();
validateUpdateRequest(request);

final Optional<Account> existingAccount = accountDao.get(accountId);
if (existingAccount.isEmpty()) {
throw new RuntimeException("Account with id = " + accountId + " not found");
}
final Account existingAccount = accountDao.get(accountId)
.orElseThrow(() -> new RuntimeException("Account with id = " + accountId + " not found"));

// // Update Stroom user
// Optional<Account> optionalUser = accountDao.get(userId);
Expand All @@ -202,9 +223,9 @@ public void update(final UpdateAccountRequest request, final int accountId) {
account.setUpdateTimeMs(System.currentTimeMillis());

// If we are reactivating account then set the reactivated time.
if ((account.isEnabled() && !existingAccount.get().isEnabled()) ||
(!account.isInactive() && existingAccount.get().isInactive()) ||
(!account.isLocked() && existingAccount.get().isLocked())) {
if ((account.isEnabled() && !existingAccount.isEnabled()) ||
(!account.isInactive() && existingAccount.isInactive()) ||
(!account.isLocked() && existingAccount.isLocked())) {
account.setReactivatedMs(System.currentTimeMillis());
}

Expand All @@ -213,9 +234,15 @@ public void update(final UpdateAccountRequest request, final int accountId) {

// Change the account password if the update request includes a new password.
if (!Strings.isNullOrEmpty(request.getPassword())
&& request.getPassword().equals(request.getConfirmPassword())) {
&& request.getPassword().equals(request.getConfirmPassword())) {
accountDao.changePassword(account.getUserId(), request.getPassword());
}

// If the fullName has changed we need to update the corresponding stroom user.
// userId obviously can't change and displayName is same as userId
if (!Objects.equals(existingAccount.getFirstName(), account.getFullName())) {
userIdentityFactory.ensureUserIdentity(createUserIdentity(account));
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import stroom.security.openid.api.OpenId;
import stroom.security.openid.api.OpenIdConfiguration;
import stroom.security.openid.api.TokenResponse;
import stroom.security.shared.AppPermission;
import stroom.security.shared.User;
import stroom.util.NullSafe;
import stroom.util.authentication.DefaultOpenIdCredentials;
Expand All @@ -35,6 +36,8 @@
import stroom.util.logging.LambdaLoggerFactory;
import stroom.util.logging.LogUtil;
import stroom.util.shared.Clearable;
import stroom.util.shared.PermissionException;
import stroom.util.shared.UserDesc;
import stroom.util.shared.UserDocRefUtil;

import jakarta.inject.Inject;
Expand Down Expand Up @@ -128,7 +131,7 @@ protected Optional<UserIdentity> mapApiIdentity(final JwtContext jwtContext,
getProcessingUser(jwtContext)
.orElseThrow(() -> new AuthenticationException(
"Expecting request to be made by processing user identity. url: "
+ request.getRequestURI()));
+ request.getRequestURI()));

final UserIdentity runAsUserIdentity = userCache.getByUuid(runAsUserUuid)
.map(User::asRef)
Expand Down Expand Up @@ -202,7 +205,7 @@ private User updateUserInfo(final User user, final JwtClaims jwtClaims) {

final Predicate<User> hasUserInfoChangedPredicate = aUser ->
!Objects.equals(displayName, aUser.getDisplayName())
|| !Objects.equals(fullName, aUser.getFullName());
|| !Objects.equals(fullName, aUser.getFullName());

if (hasUserInfoChangedPredicate.test(user)) {
synchronized (this) {
Expand Down Expand Up @@ -318,7 +321,7 @@ private UserIdentity createAuthFlowUserIdentity(final JwtClaims jwtClaims,
addTokenToRefreshManager(updatableToken);

LOGGER.info(() -> "Authenticated user " + userIdentity
+ " for sessionId " + NullSafe.get(session, HttpSession::getId));
+ " for sessionId " + NullSafe.get(session, HttpSession::getId));
return userIdentity;
}

Expand All @@ -336,15 +339,15 @@ private Optional<UserIdentity> getApiUserIdentity(final JwtContext jwtContext,
final String userUuid;

if (IdpType.TEST_CREDENTIALS.equals(openIdConfigProvider.get().getIdentityProviderType())
&& jwtContext.getJwtClaims().getAudience().contains(defaultOpenIdCredentials.getOauth2ClientId())
&& subjectId.equals(defaultOpenIdCredentials.getApiKeyUserEmail())) {
&& jwtContext.getJwtClaims().getAudience().contains(defaultOpenIdCredentials.getOauth2ClientId())
&& subjectId.equals(defaultOpenIdCredentials.getApiKeyUserEmail())) {
LOGGER.debug("Authenticating using default API key. DO NOT USE IN PRODUCTION!");
// Using default creds so just fake a user
userUuid = UUID.randomUUID().toString();
} else {
User user = getOrCreateUserBySubjectId(subjectId).orElseThrow(() ->
new AuthenticationException("Unable to find user with id: " + subjectId
+ "(displayName: " + optDisplayName + ")"));
+ "(displayName: " + optDisplayName + ")"));
user = updateUserInfo(user, jwtClaims);
userUuid = user.getUuid();
}
Expand Down Expand Up @@ -398,7 +401,7 @@ public boolean isServiceUser(final String subject,
return Optional.ofNullable(hasJwtClaims.getJwtClaims())
.map(ThrowingFunction.unchecked(jwtClaims -> {
final boolean isProcessingUser = Objects.equals(subject, jwtClaims.getSubject())
&& Objects.equals(issuer, jwtClaims.getIssuer());
&& Objects.equals(issuer, jwtClaims.getIssuer());

if (LOGGER.isDebugEnabled()) {
LOGGER.debug("Comparing subject: [{}|{}], issuer[{}|{}], result: {}",
Expand All @@ -414,7 +417,7 @@ public boolean isServiceUser(final String subject,
} else {
final String requiredIssuer = openIdConfigProvider.get().getIssuer();
final boolean isProcessingUser = Objects.equals(subject, serviceUser.getSubjectId())
&& Objects.equals(issuer, requiredIssuer);
&& Objects.equals(issuer, requiredIssuer);
if (LOGGER.isDebugEnabled()) {
LOGGER.debug("Comparing subject: [{}|{}], issuer[{}|{}], result: {}",
subject,
Expand Down Expand Up @@ -454,6 +457,30 @@ public void clear() {
cacheBySubjectId.clear();
}

@Override
public void ensureUserIdentity(final UserDesc userDesc) {
if (!securityContext.hasAppPermission(AppPermission.MANAGE_USERS_PERMISSION)) {
throw new PermissionException(securityContext.getUserRef(),
"You are not authorised to ensure a stroom user.");
}
NullSafe.requireNonNull(userDesc, UserDesc::getSubjectId, () -> "subjectId required");
final UserService userService = userServiceProvider.get();
final String newDisplayName = userDesc.getDisplayName();
final String newFullName = userDesc.getFullName();

final User user = userService.getOrCreateUser(userDesc);

if (!Objects.equals(newDisplayName, user.getDisplayName())
|| !Objects.equals(newFullName, user.getFullName())) {

LOGGER.debug("Updating user {} with displayName: '{}' and fullName: '{}'",
user, newDisplayName, newFullName);
user.setDisplayName(newDisplayName);
user.setFullName(newFullName);
userService.update(user);
}
}

@Override
public void onChange(final PermissionChangeEvent event) {
if (event.getUserRef() != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@
public interface UserService {

default User getOrCreateUser(String subjectId) {
return getOrCreateUser(UserDesc.builder().subjectId(subjectId).displayName(subjectId).build(), null);
return getOrCreateUser(UserDesc.builder(subjectId)
.displayName(subjectId)
.build(), null);
}

default User getOrCreateUser(UserDesc userDesc) {
Expand Down
30 changes: 17 additions & 13 deletions stroom-util-shared/src/main/java/stroom/util/shared/UserDesc.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ public UserDesc(@JsonProperty("subjectId") final String subjectId,
this.fullName = fullName;
}

public static UserDesc forSubjectId(final String subjectId) {
return new UserDesc(subjectId, subjectId, null);
}

public String getSubjectId() {
return subjectId;
}
Expand Down Expand Up @@ -65,40 +69,40 @@ public int hashCode() {
@Override
public String toString() {
return "ExternalUser{" +
"subjectId='" + subjectId + '\'' +
", displayName='" + displayName + '\'' +
", fullName='" + fullName + '\'' +
'}';
"subjectId='" + subjectId + '\'' +
", displayName='" + displayName + '\'' +
", fullName='" + fullName + '\'' +
'}';
}

public Builder copy() {
return new Builder(this);
}

public static Builder builder() {
return new Builder();
public static Builder builder(final String subjectId) {
return new Builder(subjectId);
}


// --------------------------------------------------------------------------------


public static class Builder {

private String subjectId;
private String displayName;
private String fullName;

private Builder() {
private Builder(final String subjectId) {
this.subjectId = Objects.requireNonNull(subjectId);
}

private Builder(final UserDesc externalUser) {
this.subjectId = externalUser.subjectId;
this.subjectId = Objects.requireNonNull(externalUser.subjectId);
this.displayName = externalUser.displayName;
this.fullName = externalUser.fullName;
}

public Builder subjectId(final String subjectId) {
this.subjectId = subjectId;
return this;
}

public Builder displayName(final String displayName) {
this.displayName = displayName;
return this;
Expand Down
Loading

0 comments on commit a86ddfc

Please sign in to comment.