Skip to content

Commit

Permalink
Refactored access to Manage and display of Manage metadata
Browse files Browse the repository at this point in the history
  • Loading branch information
oharsta committed Sep 22, 2023
1 parent 646d0af commit 3f91675
Show file tree
Hide file tree
Showing 20 changed files with 79 additions and 62 deletions.
1 change: 0 additions & 1 deletion client/src/components/UnitHeader.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {isEmpty, stopEvent} from "../utils/Utils";
import {Button, ButtonType, MenuButton} from "@surfnet/sds";
import {Link, useNavigate} from "react-router-dom";
import I18n from "../locale/I18n";
import {providerInfo} from "../utils/Manage";
import {MoreLessText} from "./MoreLessText";

export const UnitHeader = ({
Expand Down
2 changes: 1 addition & 1 deletion client/src/components/User.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export const User = ({user, other}) => {

const renderUserRole = (userRole, index) => {
const role = userRole.role;
const provider = user.providers.find(data => data.id === role.manageId) || providerInfo(null);
const provider = role.application || providerInfo(null);
const logo = provider.data.metaDataFields["logo:0:url"];
const children =
<div key={index} className={"user-role"}>
Expand Down
10 changes: 8 additions & 2 deletions client/src/locale/nl.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,14 +101,18 @@ const nl = {
expiryDays: "Expiry days"
},
roles: {
title: "Roles",
title: "Access Roles",
applicationName:"Application",
accessRole:"Name",
name: "Name",
namePlaceHolder: "The name of the role",
shortName: "Short name",
landingPage: "Website",
userRoleCount: "# Users",
landingPagePlaceHolder: "https://landingpage.com",
defaultExpiryDays: "Expiry days",
endDate: "End date",
noEndDate: "No end date",
authority: "Authority",
yourRole: "Your role",
description: "Description",
Expand Down Expand Up @@ -140,7 +144,8 @@ const nl = {
searchPlaceHolder: "Search for user roles...",
noResults: "No user roles where found",
notAllowed: "You're not allowed to delete this user role because of missing roles",
updateConfirmation: "Are you sure you want to change the end date of role {{roleName}} for {{userName}}",
updateConfirmation: "Are you sure you want to change the end date of role {{roleName}} for {{userName}}",
updateConfirmationRemoveEndDate: "Are you sure you want to remove the end date of role {{roleName}} for {{userName}}",
updateFlash: "The end date for role {{roleName}} has been updated",
deleteConfirmation: "Are you sure you want to remove this role from this user(s)?",
deleteFlash: "User role(s) have been removed",
Expand All @@ -163,6 +168,7 @@ const nl = {
newInvite: "New invite",
newGuest: "Invite guest",
invitees: "Invitees",
intendedRoles: "Roles",
inviteesPlaceholder: "Invitee email addresses",
requiredEmail: "At least one email is required",
requiredRole: "At least one role is required for an invitation",
Expand Down
4 changes: 2 additions & 2 deletions client/src/pages/InvitationForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,13 @@ export const InvitationForm = () => {
if (isUserAllowed(AUTHORITIES.MANAGER, user)) {
rolesByApplication()
.then(res => {
const markedRoles = markAndFilterRoles(user, res);
const markedRoles = markAndFilterRoles(user, res, I18n.locale);
setInitialRole(markedRoles);
setRoles(markedRoles);
})
setLoading(false);
} else {
const markedRoles = markAndFilterRoles(user, []);
const markedRoles = markAndFilterRoles(user, [], I18n.locale);
setInitialRole(markedRoles);
setRoles(markedRoles)
}
Expand Down
1 change: 1 addition & 0 deletions client/src/pages/Profile.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export const Profile = () => {
if (id) {
other(id)
.then(res => {
debugger;
setUser(res);
setLoading(false);
useAppStore.setState({
Expand Down
4 changes: 2 additions & 2 deletions client/src/pages/RoleForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export const RoleForm = () => {
if (user.superUser) {
setProviders(res[newRole ? 0 : 1]);
} else {
setProviders(user.providers);
setProviders(user.userRoles.map(userRole => userRole.role.application));
}
setNewRole(newRole);
const name = newRole ? "" : res[0].name;
Expand All @@ -65,7 +65,7 @@ export const RoleForm = () => {
{path: "/home/roles", value: I18n.t("tabs.roles")},
];
if (newRole) {
const providerOption = singleProviderToOption(user.superUser ? res[0][0] : user.providers[0]);
const providerOption = singleProviderToOption(user.superUser ? res[0][0] : user.userRoles[0].role.application);
setManagementOption(providerOption);
setRole({...role, manageId: providerOption.value, manageType: providerOption.type.toUpperCase()})
} else {
Expand Down
8 changes: 4 additions & 4 deletions client/src/tabs/Roles.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ export const Roles = () => {
if (isUserAllowed(AUTHORITIES.MANAGER, user)) {
if (!roleSearchRequired) {
rolesByApplication()
.then(res => setRoles(markAndFilterRoles(user, res)))
.then(res => setRoles(markAndFilterRoles(user, res, I18n.locale)))
setLoading(false);
}
} else {
setRoles(markAndFilterRoles(user, []))
setRoles(markAndFilterRoles(user, [], I18n.locale))
}
setLoading(false);
}, [user]);// eslint-disable-line react-hooks/exhaustive-deps
Expand Down Expand Up @@ -61,7 +61,7 @@ export const Roles = () => {
const delayedAutocomplete = debounce(query => {
searchRoles(query)
.then(res => {
setRoles(markAndFilterRoles(user, res));
setRoles(markAndFilterRoles(user, res, I18n.locale));
setMoreToShow(res.length === 15);
setNoResults(res.length === 0);
setSearching(false);
Expand All @@ -82,7 +82,7 @@ export const Roles = () => {
header: "",
mapper: role => {
return <div className="role-icon">
<img src={role.application.data.metaDataFields["logo:0:url"]} alt="logo"/>
<img src={role.logo} alt="logo"/>
</div>
}
},
Expand Down
9 changes: 4 additions & 5 deletions client/src/utils/Manage.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import {isEmpty} from "./Utils";
import I18n from "../locale/I18n";

export const singleProviderToOption = provider => {

Expand All @@ -16,13 +15,13 @@ export const providersToOptions = providers => {
return providers.map(provider => singleProviderToOption(provider));
}

export const deriveApplicationAttributes = role => {
export const deriveApplicationAttributes = (role, locale) => {
const application = role.application;
if (!isEmpty(application)) {
const metaData = application.data.metaDataFields;
role.applicationName = metaData[`name:${I18n.locale}`] || metaData["name:en"]
role.applicationOrganizationName = metaData[`OrganizationName:${I18n.locale}`] || metaData["OrganizationName:en"];
role.logo = metaData["logo:0:url"] ;
role.applicationName = metaData[`name:${locale}`] || metaData["name:en"]
role.applicationOrganizationName = metaData[`OrganizationName:${locale}`] || metaData["OrganizationName:en"];
role.logo = metaData["logo:0:url"];
}
}

Expand Down
35 changes: 20 additions & 15 deletions client/src/utils/UserRole.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import {isEmpty} from "./Utils";
import {deriveApplicationAttributes} from "./Manage";
import I18n from "../locale/I18n";

export const INVITATION_STATUS = {
OPEN: "OPEN",
Expand Down Expand Up @@ -59,7 +58,10 @@ export const allowedToEditRole = (user, role) => {

export const allowedToDeleteInvitation = (user, invitation) => {
return invitation.roles
.every(invitationRole => allowedToRenewUserRole(user, {...invitationRole, authority: invitation.intendedAuthority}))
.every(invitationRole => allowedToRenewUserRole(user, {
...invitationRole,
authority: invitation.intendedAuthority
}))
}

export const allowedToRenewUserRole = (user, userRole) => {
Expand All @@ -83,27 +85,30 @@ export const allowedToRenewUserRole = (user, userRole) => {

export const urnFromRole = (groupUrnPrefix, role) => `${groupUrnPrefix}:${role.manageId}:${role.shortName}`;

export const markAndFilterRoles = (user, allRoles) => {
export const markAndFilterRoles = (user, allRoles, locale) => {
allRoles.forEach(role => {
role.isUserRole = false;
role.label = role.name;
role.value = role.id;
deriveApplicationAttributes(role);
deriveApplicationAttributes(role, locale);
});
const userRoles = user.userRoles;
userRoles.forEach(userRole => {
userRole.isUserRole = true;
userRole.name = userRole.role.name;
userRole.label = userRole.role.name;
userRole.value = userRole.role.id;
userRole.landingPage = userRole.role.landingPage;
userRole.description = userRole.role.description;
userRole.defaultExpiryDays = userRole.role.defaultExpiryDays;
userRole.eduIDOnly = userRole.role.eduIDOnly;
userRole.enforceEmailEquality = userRole.role.enforceEmailEquality;
userRole.applicationName = userRole.role.applicationName;
userRole.applicationOrganizationName = userRole.role.applicationOrganizationName;
userRole.logo = userRole.role.logo;
const role = userRole.role;
deriveApplicationAttributes(role, locale);
userRole.name = role.name;
userRole.label = role.name;
userRole.value = role.id;
userRole.landingPage = role.landingPage;
userRole.description = role.description;
userRole.defaultExpiryDays = role.defaultExpiryDays;
userRole.eduIDOnly = role.eduIDOnly;
userRole.enforceEmailEquality = role.enforceEmailEquality;
userRole.applicationName = role.applicationName;
userRole.applicationOrganizationName = role.applicationOrganizationName;
userRole.logo = role.logo;
userRole.userRoleCount = role.userRoleCount;
})

return allRoles
Expand Down
21 changes: 6 additions & 15 deletions server/src/main/java/access/api/UserController.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
import access.manage.ManageIdentifier;
import access.manage.Manage;
import access.model.Authority;
import access.model.Role;
import access.model.User;
import access.model.UserRole;
import access.repository.UserRepository;
import access.security.UserPermissions;
import com.fasterxml.jackson.core.JsonProcessingException;
Expand Down Expand Up @@ -85,9 +87,8 @@ public ResponseEntity<Config> config(User user) {
@GetMapping("me")
public ResponseEntity<User> me(@Parameter(hidden = true) User user) {
LOG.debug("/me");
List<Map<String, Object>> providers = getProviders(user);
user.setProviders(providers);

List<Role> roles = user.getUserRoles().stream().map(UserRole::getRole).toList();
manage.deriveRemoteApplications(roles);
return ResponseEntity.ok(user);
}

Expand All @@ -96,9 +97,9 @@ public ResponseEntity<User> details(@PathVariable("id") Long id, @Parameter(hidd
LOG.debug("/me");
UserPermissions.assertSuperUser(user);
User other = userRepository.findById(id).orElseThrow(NotFoundException::new);
List<Map<String, Object>> providers = getProviders(other);
other.setProviders(providers);

List<Role> roles = user.getUserRoles().stream().map(UserRole::getRole).toList();
manage.deriveRemoteApplications(roles);
return ResponseEntity.ok(other);
}

Expand Down Expand Up @@ -152,16 +153,6 @@ public ResponseEntity<Map<String, Integer>> error(@RequestBody Map<String, Objec
return Results.createResult();
}

private List<Map<String, Object>> getProviders(User user) {
return user.getUserRoles().stream()
.map(userRole -> new ManageIdentifier(userRole.getRole().getManageId(), userRole.getRole().getManageType()))
//Prevent unnecessary round-trips to Manage
.collect(Collectors.toSet())
.stream()
.map(identity -> manage.providerById(identity.entityType(), identity.id()))
.toList();
}

private void verifyMissingAttributes(User user, Config result) {
List<String> missingAttributes = new ArrayList<>();
if (!StringUtils.hasText(user.getSub())) {
Expand Down
5 changes: 5 additions & 0 deletions server/src/main/java/access/manage/Manage.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package access.manage;

import access.model.Role;
import org.springframework.util.CollectionUtils;

import java.util.*;
import java.util.stream.Collectors;
Expand All @@ -26,6 +27,10 @@ default List<Map<String, Object>> addIdentifierAlias(List<Map<String, Object>> p
}

default Map<String, Object> addIdentifierAlias(Map<String, Object> provider) {
//Defensive mostly because of tests
if (CollectionUtils.isEmpty(provider)) {
return provider;
}
Object id = provider.get("id");
if (id != null) {
provider.put("_id", id);
Expand Down
3 changes: 0 additions & 3 deletions server/src/main/java/access/model/User.java
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,6 @@ public class User implements Serializable, Provisionable {
@JsonIgnore
private Set<RemoteProvisionedUser> remoteProvisionedUsers = new HashSet<>();

@Transient
private List<Map<String, Object>> providers;

public User(Map<String, Object> attributes) {
this(false, attributes);
}
Expand Down
4 changes: 2 additions & 2 deletions server/src/main/java/access/repository/RoleRepository.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
@Repository
public interface RoleRepository extends JpaRepository<Role, Long> {

@Query(value = "SELECT * FROM roles WHERE MATCH (name, description) against (?1 IN BOOLEAN MODE) " +
"AND id > 0 LIMIT ?2",
@Query(value = "SELECT *, (SELECT COUNT(*) FROM user_roles ur WHERE ur.role_id=r.id) as userRoleCount " +
"FROM roles r WHERE MATCH (name, description) against (?1 IN BOOLEAN MODE) AND id > 0 LIMIT ?2",
nativeQuery = true)
List<Role> search(String keyWord, int limit);

Expand Down
8 changes: 8 additions & 0 deletions server/src/test/java/access/api/RoleControllerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,8 @@ void nameExists() throws Exception {
@Test
void rolesByApplication() throws Exception {
AccessCookieFilter accessCookieFilter = openIDConnectFlow("/api/v1/users/login", MANAGE_SUB);
super.stubForManageProviderByEntityID(EntityType.SAML20_SP, "https://wiki");

List<Role> roles = given()
.when()
.filter(accessCookieFilter.cookieFilter())
Expand All @@ -170,6 +172,9 @@ void rolesByApplication() throws Exception {
@Test
void rolesByApplicationSuperUser() throws Exception {
AccessCookieFilter accessCookieFilter = openIDConnectFlow("/api/v1/users/login", SUPER_SUB);
super.stubForManageProviderByEntityID(EntityType.SAML20_SP, "https://wiki");
super.stubForManageProviderByEntityID(EntityType.OIDC10_RP, "https://calendar");

List<Role> roles = given()
.when()
.filter(accessCookieFilter.cookieFilter())
Expand Down Expand Up @@ -228,6 +233,9 @@ void deleteRole() throws Exception {
@Test
void search() throws Exception {
AccessCookieFilter accessCookieFilter = openIDConnectFlow("/api/v1/users/login", SUPER_SUB);
super.stubForManageProviderByEntityID(EntityType.SAML20_SP, "https://wiki");
super.stubForManageProviderByEntityID(EntityType.OIDC10_RP, "https://calendar");

List<Role> roles = given()
.when()
.filter(accessCookieFilter.cookieFilter())
Expand Down
16 changes: 10 additions & 6 deletions server/src/test/java/access/api/UserControllerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ void meWithOauth2Login() throws Exception {
void meWithRoles() throws Exception {
AccessCookieFilter accessCookieFilter = openIDConnectFlow("/api/v1/users/me", INVITER_SUB);

String body = objectMapper.writeValueAsString(localManage.providerById(EntityType.OIDC10_RP, "5"));
stubFor(get(urlPathMatching("/manage/api/internal/metadata/oidc10_rp/5")).willReturn(aResponse()
String body = objectMapper.writeValueAsString(List.of(localManage.providerById(EntityType.OIDC10_RP, "5")));
stubFor(get(urlPathMatching("/manage/api/internal/rawSearch/oidc10_rp")).willReturn(aResponse()
.withHeader("Content-Type", "application/json")
.withBody(body)));

Expand All @@ -110,9 +110,9 @@ void meWithRoles() throws Exception {
List<String> roleNames = List.of("Mail", "Calendar").stream().sorted().toList();

assertEquals(roleNames, user.getUserRoles().stream().map(userRole -> userRole.getRole().getName()).sorted().toList());
assertEquals(1, user.getProviders().size());
assertEquals("5", user.getProviders().get(0).get("id"));
assertEquals("5", user.getProviders().get(0).get("_id"));
List<Object> applicationIdentifiers = user.getUserRoles().stream()
.map(userRole -> userRole.getRole().getApplication().get("id")).sorted().toList();
assertEquals(List.of("5", "5"), applicationIdentifiers);
}

@Test
Expand Down Expand Up @@ -146,6 +146,9 @@ void meWithImpersonation() throws Exception {

User manager = userRepository.findBySubIgnoreCase(MANAGE_SUB).get();
stubForManageProviderById(EntityType.SAML20_SP, "1");
super.stubForManageProviderByEntityID(EntityType.SAML20_SP, "https://wiki");
super.stubForManageProviderByEntityID(EntityType.OIDC10_RP, "https://calendar");


User user = given()
.when()
Expand All @@ -163,7 +166,8 @@ void meWithNotAllowedImpersonation() throws Exception {
AccessCookieFilter accessCookieFilter = openIDConnectFlow("/api/v1/users/login", MANAGE_SUB);

User guest = userRepository.findBySubIgnoreCase(GUEST_SUB).get();
stubForManageProviderById(EntityType.SAML20_SP, "1");
super.stubForManageProviderByEntityID(EntityType.SAML20_SP, "https://wiki");
super.stubForManageProviderByEntityID(EntityType.OIDC10_RP, "https://calendar");

User user = given()
.when()
Expand Down
2 changes: 1 addition & 1 deletion welcome/src/components/RoleCard.scss
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
img, svg {
height: 56px;
width: auto;
margin-right: 35px;
margin: 0 35px auto 0;
border-radius: $br;

@media (max-width: 680px) {
Expand Down
2 changes: 1 addition & 1 deletion welcome/src/components/User.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export const User = ({user, invitationRoles = []}) => {

const renderUserRole = (userRole, index) => {
const role = userRole.role;
const provider = providerInfo(user.providers.find(data => data.id === role.manageId));
const provider = providerInfo(role.application);
return (
<RoleCard role={role} provider={provider} index={index}/>
);
Expand Down
Loading

0 comments on commit 3f91675

Please sign in to comment.