-
Notifications
You must be signed in to change notification settings - Fork 6
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
Bumps sirius-biz and applies breaking changes #2064
Changes from all commits
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 |
---|---|---|
|
@@ -181,7 +181,7 @@ protected List<T> querySAMLTenants() { | |
|
||
private void verifyUser(SAMLResponse response, UserInfo user) { | ||
U account = user.getUserObject(getUserClass()); | ||
T tenant = account.getTenant().fetchValue(); | ||
T tenant = account.getTenant().fetchCachedValue(); | ||
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. Question: Wouldn't it be better to use the cache of the 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. It is. Extrapolates the purpose of the change/ticket though which requires additional/special testing. |
||
|
||
if (!checkIssuer(tenant, response)) { | ||
throw Exceptions.createHandled().withSystemErrorMessage("SAML Error: Issuer mismatch!").handle(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -407,7 +407,7 @@ public void selectTenant(final WebContext webContext, String tenantId) { | |
.causedByUser(account.getUniqueName(), account.getUserAccountData().getLogin().getUsername()) | ||
.forUser(account.getUniqueName(), account.getUserAccountData().getLogin().getUsername()) | ||
.forTenant(account.getTenant().getIdAsString(), | ||
account.getTenant().fetchValue().getTenantData().getName()) | ||
account.getTenant().fetchCachedValue().getTenantData().getName()) | ||
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. see above |
||
.log(); | ||
|
||
webContext.setSessionValue(UserContext.getCurrentScope().getScopeId() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -453,7 +453,7 @@ public UserInfo findUserByName(@Nullable WebContext webContext, String user) { | |
U account = optionalAccount.get(); | ||
|
||
userAccountCache.put(account.getUniqueName(), account); | ||
tenantsCache.put(account.getTenant().fetchValue().getIdAsString(), account.getTenant().fetchValue()); | ||
tenantsCache.put(account.getTenant().fetchCachedValue().getIdAsString(), account.getTenant().fetchCachedValue()); | ||
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. Don't think its a good idea to fill a cache with a cached value |
||
rolesCache.remove(account.getUniqueName()); | ||
configCache.remove(account.getUniqueName()); | ||
|
||
|
@@ -525,7 +525,7 @@ private UserInfo verifyIpRange(WebContext webContext, UserInfo info) { | |
return defaultUser; | ||
} | ||
|
||
T tenant = account.getTenant().fetchValue(); | ||
T tenant = account.getTenant().fetchCachedValue(); | ||
|
||
if (tenant != null && !tenant.getTenantData().matchesIPRange(webContext)) { | ||
return createUserWithLimitedRoles(info, tenant.getTenantData().getRolesToKeepAsSet()); | ||
|
@@ -569,7 +569,8 @@ protected U fetchAccount(@Nonnull String accountId) { | |
return null; | ||
} | ||
userAccountCache.put(account.getUniqueName(), account); | ||
tenantsCache.put(account.getTenant().fetchValue().getIdAsString(), account.getTenant().fetchValue()); | ||
tenantsCache.put(account.getTenant().fetchCachedValue().getIdAsString(), | ||
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. see above |
||
account.getTenant().fetchCachedValue()); | ||
rolesCache.remove(account.getUniqueName()); | ||
configCache.remove(account.getUniqueName()); | ||
|
||
|
@@ -604,7 +605,7 @@ private UserInfo asUserWithRoles(U account, Set<String> roles, @Nullable Supplie | |
return UserInfo.Builder.createUser(account.getUniqueName()) | ||
.withUsername(account.getUserAccountData().getLogin().getUsername()) | ||
.withTenantId(String.valueOf(account.getTenant().getId())) | ||
.withTenantName(account.getTenant().fetchValue().getTenantData().getName()) | ||
.withTenantName(account.getTenant().fetchCachedValue().getTenantData().getName()) | ||
.withLanguage(computeLanguage(null, account.getUniqueName())) | ||
.withPermissions(roles) | ||
.withSettingsSupplier(user -> getUserSettings(getScopeSettings(), user)) | ||
|
@@ -641,7 +642,7 @@ public UserInfo findUserByCredentials(@Nullable WebContext webContext, String us | |
.getLogin() | ||
.getLastExternalLogin(), | ||
account.getTenant() | ||
.fetchValue() | ||
.fetchCachedValue() | ||
.getTenantData() | ||
.getExternalLoginIntervalDays())) { | ||
completeAuditLogForUser(auditLog.negative("AuditLog.externalLoginRequired"), account); | ||
|
@@ -673,7 +674,7 @@ public UserInfo findUserByCredentials(@Nullable WebContext webContext, String us | |
auditLog.negative("AuditLog.loginRejected") | ||
.forUser(account.getUniqueName(), account.getUserAccountData().getLogin().getUsername()) | ||
.forTenant(String.valueOf(account.getTenant().getId()), | ||
account.getTenant().fetchValue().getTenantData().getName()) | ||
account.getTenant().fetchCachedValue().getTenantData().getName()) | ||
.log(); | ||
|
||
return null; | ||
|
@@ -718,7 +719,7 @@ protected void completeAuditLogForUser(AuditLog.AuditLogBuilder builder, U accou | |
builder.causedByUser(account.getUniqueName(), account.getUserAccountData().getLogin().getUsername()) | ||
.forUser(account.getUniqueName(), account.getUserAccountData().getLogin().getUsername()) | ||
.forTenant(String.valueOf(account.getTenant().getId()), | ||
account.getTenant().fetchValue().getTenantData().getName()) | ||
account.getTenant().fetchCachedValue().getTenantData().getName()) | ||
.log(); | ||
} | ||
|
||
|
@@ -791,7 +792,7 @@ protected U getUserObject(UserInfo userInfo) { | |
protected UserSettings getUserSettings(UserSettings scopeSettings, UserInfo userInfo) { | ||
U user = userInfo.getUserObject(getUserClass()); | ||
Config userAccountConfig = user.getUserAccountData().getPermissions().getConfig(); | ||
Config tenantConfig = user.getTenant().fetchValue().getTenantData().getConfig(); | ||
Config tenantConfig = user.getTenant().fetchCachedValue().getTenantData().getConfig(); | ||
|
||
if (userAccountConfig == null) { | ||
if (tenantConfig == null) { | ||
|
@@ -852,7 +853,7 @@ protected boolean isUserStillValid(String userId, WebContext webContext) { | |
} | ||
|
||
LoginData loginData = user.getUserAccountData().getLogin(); | ||
TenantData tenantData = user.getTenant().fetchValue().getTenantData(); | ||
TenantData tenantData = user.getTenant().fetchCachedValue().getTenantData(); | ||
|
||
if (loginData.isAccountLocked()) { | ||
return false; | ||
|
@@ -966,7 +967,7 @@ protected Set<String> computeRoles(WebContext webContext, String accountUniqueNa | |
} | ||
|
||
Set<String> roles = computeRoles(user, | ||
user.getTenant().fetchValue(), | ||
user.getTenant().fetchCachedValue(), | ||
Strings.areEqual(systemTenant, String.valueOf(user.getTenant().getId()))); | ||
|
||
rolesCache.put(accountUniqueName, Tuple.create(roles, user.getTenant().getUniqueObjectName())); | ||
|
@@ -1041,7 +1042,7 @@ protected String computeLanguage(WebContext webContext, String userId) { | |
return NLS.getDefaultLanguage(); | ||
} | ||
return Strings.firstFilled(userAccount.getUserAccountData().getLanguage().getValue(), | ||
userAccount.getTenant().fetchValue().getTenantData().getLanguage().getValue(), | ||
userAccount.getTenant().fetchCachedValue().getTenantData().getLanguage().getValue(), | ||
NLS.getDefaultLanguage()); | ||
} | ||
} |
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.
Question: Doesn't use of the ref-cache here potentially lead to trying to delete a blob that may be already deleted otherwise?
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.
True, will review the places.