From dd441c56772e44a166441d849cddbef4f2b0e070 Mon Sep 17 00:00:00 2001 From: Ryan Mathis Date: Wed, 23 Jun 2021 10:14:23 -0700 Subject: [PATCH 1/9] Fixed issue with loading sources --- ui/src/app/dashboard/view/Dashboard.js | 2 +- ui/src/app/metadata/hooks/api.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/src/app/dashboard/view/Dashboard.js b/ui/src/app/dashboard/view/Dashboard.js index 5869e5695..d7892f028 100644 --- a/ui/src/app/dashboard/view/Dashboard.js +++ b/ui/src/app/dashboard/view/Dashboard.js @@ -40,7 +40,7 @@ export function Dashboard () { } async function loadSources() { - const s = sourceLoader.get(); + const s = await sourceLoader.get(); if (response.ok) { setSources(s); } diff --git a/ui/src/app/metadata/hooks/api.js b/ui/src/app/metadata/hooks/api.js index d951b66a4..f628e847d 100644 --- a/ui/src/app/metadata/hooks/api.js +++ b/ui/src/app/metadata/hooks/api.js @@ -24,7 +24,7 @@ export function getMetadataPath(type) { } export function useNonAdminSources() { - return useFetch(`/${getMetadataPath('source')}/disabledNonAdmin`); + return useFetch(`${API_BASE_PATH}${getMetadataPath('source')}/disabledNonAdmin`); } export function getMetadataListPath(type) { From 3ca04e0da933ad8aca2ade6e5b00b7ca48e65d0b Mon Sep 17 00:00:00 2001 From: Ryan Mathis Date: Wed, 23 Jun 2021 10:34:59 -0700 Subject: [PATCH 2/9] Fixed script validation --- ui/src/app/form/component/fields/FilterTargetField.js | 6 +++--- .../domain/filter/EntityAttributesFilterDefinition.js | 7 +++++++ .../app/metadata/domain/filter/NameIdFilterDefinition.js | 7 +++++++ 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/ui/src/app/form/component/fields/FilterTargetField.js b/ui/src/app/form/component/fields/FilterTargetField.js index 2bfd8484d..c273f3462 100644 --- a/ui/src/app/form/component/fields/FilterTargetField.js +++ b/ui/src/app/form/component/fields/FilterTargetField.js @@ -193,13 +193,13 @@ const FilterTargetField = ({ className="codearea form-control" rows="8" onChange={({ target: { value } }) => handleTextChange(value)} - html={ selectedTarget[0] } + html={ selectedTarget[0] ? selectedTarget[0] : '' } innerRef={ref} dangerouslySetInnerHTML={true}> - + {!selectedTarget[0] && Required for Scripts - + } } {targetType === 'REGEX' && <> diff --git a/ui/src/app/metadata/domain/filter/EntityAttributesFilterDefinition.js b/ui/src/app/metadata/domain/filter/EntityAttributesFilterDefinition.js index 727df9f8f..036557706 100644 --- a/ui/src/app/metadata/domain/filter/EntityAttributesFilterDefinition.js +++ b/ui/src/app/metadata/domain/filter/EntityAttributesFilterDefinition.js @@ -53,6 +53,13 @@ export const EntityAttributesFilterWizard = { errors.entityAttributesFilterTarget.value.addError('message.invalid-regex-pattern'); } } + + if (formData?.entityAttributesFilterTarget?.entityAttributesFilterTargetType === 'CONDITION_SCRIPT') { + const { entityAttributesFilterTarget: { value } } = formData; + if (!value[0]) { + errors.entityAttributesFilterTarget.value.addError('message.required-for-scripts'); + } + } return errors; } }, diff --git a/ui/src/app/metadata/domain/filter/NameIdFilterDefinition.js b/ui/src/app/metadata/domain/filter/NameIdFilterDefinition.js index ff16383e9..f092b5813 100644 --- a/ui/src/app/metadata/domain/filter/NameIdFilterDefinition.js +++ b/ui/src/app/metadata/domain/filter/NameIdFilterDefinition.js @@ -41,6 +41,13 @@ export const NameIDFilterWizard = { errors.nameIdFormatFilterTarget.value.addError('message.invalid-regex-pattern'); } } + + if (formData?.nameIdFormatFilterTarget?.nameIdFormatFilterTargetType === 'CONDITION_SCRIPT') { + const { nameIdFormatFilterTarget: { value } } = formData; + if (!value[0]) { + errors.nameIdFormatFilterTarget.value.addError('message.required-for-scripts'); + } + } return errors; } }, From 93f6527a99f2aaba5f48d639089b53535bde8336 Mon Sep 17 00:00:00 2001 From: Ryan Mathis Date: Wed, 23 Jun 2021 12:16:38 -0700 Subject: [PATCH 3/9] Fixed script editor --- ui/package-lock.json | 21 +++---------- ui/package.json | 2 +- .../component/fields/FilterTargetField.js | 31 ++++++++++--------- ui/src/theme/project/forms.scss | 18 +++++++---- 4 files changed, 35 insertions(+), 37 deletions(-) diff --git a/ui/package-lock.json b/ui/package-lock.json index ab86d547d..8cbf388c2 100644 --- a/ui/package-lock.json +++ b/ui/package-lock.json @@ -13213,22 +13213,6 @@ } } }, - "react-contenteditable": { - "version": "3.3.5", - "resolved": "https://registry.npmjs.org/react-contenteditable/-/react-contenteditable-3.3.5.tgz", - "integrity": "sha512-38A7hlRQfb2KQAQT0kIJC2YlQUU7jcyYM4eh1fj6kAYb3Hmk6hHlr0snelyxVSpPXjPdFllrnSsPkzUS5AtrEA==", - "requires": { - "fast-deep-equal": "^2.0.1", - "prop-types": "^15.7.1" - }, - "dependencies": { - "fast-deep-equal": { - "version": "2.0.1", - "resolved": "https://registry.npmjs.org/fast-deep-equal/-/fast-deep-equal-2.0.1.tgz", - "integrity": "sha1-ewUhjd+WZ79/Nwv3/bLLFf3Qqkk=" - } - } - }, "react-dev-utils": { "version": "11.0.4", "resolved": "https://registry.npmjs.org/react-dev-utils/-/react-dev-utils-11.0.4.tgz", @@ -13607,6 +13591,11 @@ "prop-types": "^15.7.2" } }, + "react-simple-code-editor": { + "version": "0.11.0", + "resolved": "https://registry.npmjs.org/react-simple-code-editor/-/react-simple-code-editor-0.11.0.tgz", + "integrity": "sha512-xGfX7wAzspl113ocfKQAR8lWPhavGWHL3xSzNLeseDRHysT+jzRBi/ExdUqevSMos+7ZtdfeuBOXtgk9HTwsrw==" + }, "react-transition-group": { "version": "4.4.1", "resolved": "https://registry.npmjs.org/react-transition-group/-/react-transition-group-4.4.1.tgz", diff --git a/ui/package.json b/ui/package.json index 1046146a3..80e8551f8 100644 --- a/ui/package.json +++ b/ui/package.json @@ -20,12 +20,12 @@ "react": "^17.0.2", "react-bootstrap": "^1.5.2", "react-bootstrap-typeahead": "^5.1.4", - "react-contenteditable": "^3.3.5", "react-dom": "^17.0.2", "react-hook-form": "^7.5.2", "react-infinite-scroll-component": "^6.1.0", "react-router-dom": "^5.2.0", "react-scroll": "^1.8.2", + "react-simple-code-editor": "^0.11.0", "use-http": "^1.0.20", "use-query-params": "^1.2.2", "web-vitals": "^1.0.1" diff --git a/ui/src/app/form/component/fields/FilterTargetField.js b/ui/src/app/form/component/fields/FilterTargetField.js index c273f3462..959306b71 100644 --- a/ui/src/app/form/component/fields/FilterTargetField.js +++ b/ui/src/app/form/component/fields/FilterTargetField.js @@ -6,12 +6,15 @@ import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; import { faAsterisk, faCaretDown, faCaretUp, faEye, faEyeSlash, faPlus, faSpinner, faTrash } from '@fortawesome/free-solid-svg-icons'; import { useTranslator } from '../../../i18n/hooks'; import { InfoIcon } from '../InfoIcon'; -import ContentEditable from 'react-contenteditable'; import { AsyncTypeahead } from 'react-bootstrap-typeahead'; import useFetch from 'use-http'; import queryString from 'query-string'; import API_BASE_PATH from '../../../App.constant'; import isNil from 'lodash/isNil'; +import Editor from 'react-simple-code-editor'; +// import { highlight, languages } from 'prismjs/components/prism-core'; +// import 'prismjs/components/prism-clike'; +// import 'prismjs/components/prism-javascript'; import { FilterTargetPreview } from '../../../metadata/hoc/FilterTargetPreview'; @@ -89,8 +92,6 @@ const FilterTargetField = ({ const displayType = selectedType?.label || ''; const targetType = selectedType?.value || null; - const ref = React.useRef(selectedTarget[0]); - var handleTextChange = function (value) { setSelectedTarget([value]); }; @@ -187,20 +188,22 @@ const FilterTargetField = ({ } { targetType === 'CONDITION_SCRIPT' && - <> - handleTextChange(value)} - html={ selectedTarget[0] ? selectedTarget[0] : '' } - innerRef={ref} - dangerouslySetInnerHTML={true}> - +
+ code} + onValueChange={(code) => handleTextChange(code)} + padding={10} + className={`codearea border rounded ${!selectedTarget[0] && 'is-invalid border-danger'}`} + style={{ + fontFamily: 'monospace', + fontSize: 15, + }}> + {!selectedTarget[0] && Required for Scripts } - } +
} {targetType === 'REGEX' && <> pre, &.is-invalid > textarea { + outline-color: transparent; } } @@ -123,3 +128,4 @@ mark { border-left: 0px; } } + From edd222fc492670302349ac571910a83882ec15a8 Mon Sep 17 00:00:00 2001 From: Ryan Mathis Date: Wed, 23 Jun 2021 13:25:03 -0700 Subject: [PATCH 4/9] Fixed issues with filters --- backend/src/main/resources/nameid-filter.schema.json | 2 +- .../metadata/component/properties/PrimitiveProperty.js | 6 +++++- .../app/metadata/component/properties/PropertyValue.js | 5 ++++- ui/src/app/metadata/hooks/api.js | 6 +++++- ui/src/app/metadata/new/NewFilter.js | 7 ++++++- ui/src/app/metadata/view/EditFilter.js | 10 +++++++--- ui/src/app/metadata/view/MetadataUpload.js | 2 -- 7 files changed, 28 insertions(+), 10 deletions(-) diff --git a/backend/src/main/resources/nameid-filter.schema.json b/backend/src/main/resources/nameid-filter.schema.json index d6d02c84f..0865d8f89 100644 --- a/backend/src/main/resources/nameid-filter.schema.json +++ b/backend/src/main/resources/nameid-filter.schema.json @@ -21,7 +21,7 @@ "type": "object", "properties": { "nameIdFormatFilterTargetType": { - "title": "", + "title": "label.filter-target-type", "type": "string", "default": "ENTITY", "enum": [ diff --git a/ui/src/app/metadata/component/properties/PrimitiveProperty.js b/ui/src/app/metadata/component/properties/PrimitiveProperty.js index f86a798b7..96440749f 100644 --- a/ui/src/app/metadata/component/properties/PrimitiveProperty.js +++ b/ui/src/app/metadata/component/properties/PrimitiveProperty.js @@ -9,6 +9,10 @@ export function PrimitiveProperty ({ property, columns }) { const width = usePropertyWidth(columns); + const getValue = (v) => { + return property.enum && property.enumNames ? property.enumNames[property.enum.indexOf(v)] : v; + } + return (
{property.differences && Changed: } @@ -21,7 +25,7 @@ export function PrimitiveProperty ({ property, columns }) { { property.name } {property.value.map((v, valIdx) => - + ) }
diff --git a/ui/src/app/metadata/component/properties/PropertyValue.js b/ui/src/app/metadata/component/properties/PropertyValue.js index 807c86e61..0476fdb01 100644 --- a/ui/src/app/metadata/component/properties/PropertyValue.js +++ b/ui/src/app/metadata/component/properties/PropertyValue.js @@ -3,6 +3,7 @@ import Popover from 'react-bootstrap/Popover'; import OverlayTrigger from 'react-bootstrap/OverlayTrigger'; import { usePropertyWidth } from './hooks'; +import Translate from '../../../i18n/components/translate'; export function PropertyValue ({ name, value, columns, className }) { @@ -20,7 +21,9 @@ export function PropertyValue ({ name, value, columns, className }) { className={`d-block text-truncate ${className}`} role="definition" style={columns ? { width } : {}}> - {value !== undefined ? value.toString() : (value === false) ? value.toString() : '-'} + + {value !== undefined ? value.toString() : (value === false) ? value.toString() : '-'} + : -} diff --git a/ui/src/app/metadata/hooks/api.js b/ui/src/app/metadata/hooks/api.js index d951b66a4..f5b64c25c 100644 --- a/ui/src/app/metadata/hooks/api.js +++ b/ui/src/app/metadata/hooks/api.js @@ -109,7 +109,11 @@ export function useMetadataUpdater (path, current) { })); }); } - return Promise.resolve(req); + if (response.ok) { + return Promise.resolve(req); + } else { + return Promise.reject(req); + } } return { diff --git a/ui/src/app/metadata/new/NewFilter.js b/ui/src/app/metadata/new/NewFilter.js index 5fce5d31f..4aacb1574 100644 --- a/ui/src/app/metadata/new/NewFilter.js +++ b/ui/src/app/metadata/new/NewFilter.js @@ -9,12 +9,14 @@ import { MetadataForm } from '../hoc/MetadataFormContext'; import { MetadataSchema } from '../hoc/MetadataSchema'; import { useMetadataFilters, useMetadataFilterTypes } from '../hooks/api'; import { MetadataFilterTypeSelector } from '../wizard/MetadataFilterTypeSelector'; +import { createNotificationAction, NotificationTypes, useNotificationDispatcher } from '../../notifications/hoc/Notifications'; export function NewFilter() { const { id, section } = useParams(); const history = useHistory(); const types = useMetadataFilterTypes(); + const dispatch = useNotificationDispatcher(); const { post, response, loading } = useMetadataFilters(id, {}); @@ -22,9 +24,12 @@ export function NewFilter() { async function save(metadata) { - await post(``, metadata); + const resp = await post(``, metadata); if (response.ok) { + dispatch(createNotificationAction('Filter saved')); gotoDetail({ refresh: true }); + } else { + dispatch(createNotificationAction(resp.cause, NotificationTypes.DANGER)); } }; diff --git a/ui/src/app/metadata/view/EditFilter.js b/ui/src/app/metadata/view/EditFilter.js index bae6fa78e..5ac69fb8c 100644 --- a/ui/src/app/metadata/view/EditFilter.js +++ b/ui/src/app/metadata/view/EditFilter.js @@ -11,9 +11,12 @@ import { MetadataSchema } from '../hoc/MetadataSchema'; import { getMetadataPath, useMetadataUpdater } from '../hooks/api'; import { useMetadataFilterObject } from '../hoc/MetadataFilterSelector'; import API_BASE_PATH from '../../App.constant'; +import { createNotificationAction, NotificationTypes, useNotificationDispatcher } from '../../notifications/hoc/Notifications'; export function EditFilter() { + const dispatch = useNotificationDispatcher(); + const { id, filterId } = useParams(); const filter = useMetadataFilterObject(); const history = useHistory(); @@ -33,10 +36,11 @@ export function EditFilter() { function save(metadata) { setBlocking(false); - update(``, metadata).then(() => { + update(``, metadata).then((resp) => { + dispatch(createNotificationAction('Filter saved')); gotoDetail({ refresh: true }); - }).catch(() => { - window.location.reload(); + }).catch((error) => { + dispatch(createNotificationAction(error.cause, NotificationTypes.DANGER)); }); }; diff --git a/ui/src/app/metadata/view/MetadataUpload.js b/ui/src/app/metadata/view/MetadataUpload.js index d00be91fe..76ab28c5a 100644 --- a/ui/src/app/metadata/view/MetadataUpload.js +++ b/ui/src/app/metadata/view/MetadataUpload.js @@ -22,8 +22,6 @@ export function MetadataUpload() { async function save({serviceProviderName, file, url}) { - console.log(serviceProviderName, file); - setSaving(true); const f = file?.length > 0 ? file[0] : null; From c1113339db07b526e55da24a26017ebc59fae97a Mon Sep 17 00:00:00 2001 From: chasegawa Date: Wed, 23 Jun 2021 16:36:15 -0700 Subject: [PATCH 5/9] SHIBUI-1848 Basic controller and supporting items for CRUD operations around user groups --- .../admin/ui/controller/GroupController.java | 115 ++++++++++++++ .../shibboleth/admin/ui/domain/Group.java | 26 ++++ .../admin/ui/repository/GroupRepository.java | 16 ++ .../admin/ui/service/GroupServiceImpl.java | 36 +++++ .../admin/ui/service/IGroupService.java | 17 +++ .../ui/repository/GroupRepositoryTests.groovy | 144 ++++++++++++++++++ 6 files changed, 354 insertions(+) create mode 100644 backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/GroupController.java create mode 100644 backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/Group.java create mode 100644 backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/repository/GroupRepository.java create mode 100644 backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/GroupServiceImpl.java create mode 100644 backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/IGroupService.java create mode 100644 backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/repository/GroupRepositoryTests.groovy diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/GroupController.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/GroupController.java new file mode 100644 index 000000000..bb54d113b --- /dev/null +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/GroupController.java @@ -0,0 +1,115 @@ +package edu.internet2.tier.shibboleth.admin.ui.controller; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; +import org.springframework.stereotype.Controller; +import org.springframework.transaction.annotation.Transactional; +import org.springframework.web.bind.annotation.DeleteMapping; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.PostMapping; +import org.springframework.web.bind.annotation.PutMapping; +import org.springframework.web.bind.annotation.RequestBody; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.servlet.support.ServletUriComponentsBuilder; + +import edu.internet2.tier.shibboleth.admin.ui.domain.Group; +import edu.internet2.tier.shibboleth.admin.ui.service.IGroupService; + +@Controller +@RequestMapping(value = "/api/groups") +public class GroupController { + @Autowired + private IGroupService groupService; + + @PostMapping + @Transactional + public ResponseEntity create(@RequestBody Group group) { + // If already defined, we can't create a new one, nor will this call update the definition + Group g = groupService.find(group.getResourceId()); + + if (g != null) { + HttpHeaders headers = new HttpHeaders(); + headers.setLocation(ServletUriComponentsBuilder.fromCurrentServletMapping().path("/api/groups").build().toUri()); + + return ResponseEntity.status(HttpStatus.METHOD_NOT_ALLOWED).headers(headers) + .body(new ErrorResponse(String.valueOf(HttpStatus.METHOD_NOT_ALLOWED.value()), + String.format("The group with resource id: [%s] and name: [%s] already exists.", + group.getResourceId(), group.getName()))); + } + + Group result = groupService.createOrUpdateGroup(g); + return ResponseEntity.status(HttpStatus.CREATED).body(result); + } + + @PutMapping + @Transactional + public ResponseEntity update(@RequestBody Group group) { + Group g = groupService.find(group.getResourceId()); + + if (g == null) { + HttpHeaders headers = new HttpHeaders(); + headers.setLocation(ServletUriComponentsBuilder.fromCurrentServletMapping().path("/api/groups").build().toUri()); + + return ResponseEntity.status(HttpStatus.NOT_FOUND).headers(headers) + .body(new ErrorResponse(String.valueOf(HttpStatus.NOT_FOUND.value()), + String.format("Unable to find group with resource id: [%s] and name: [%s]", + group.getResourceId(), group.getName()))); + } + + Group result = groupService.createOrUpdateGroup(g); + return ResponseEntity.ok(result); + } + + @GetMapping + @Transactional(readOnly = true) + public ResponseEntity getAll() { + return ResponseEntity.ok(groupService.findAll()); + } + + @GetMapping("/{resourceId}") + @Transactional(readOnly = true) + public ResponseEntity getOne(@PathVariable String resourceId) { + Group g = groupService.find(resourceId); + + if (g == null) { + HttpHeaders headers = new HttpHeaders(); + headers.setLocation(ServletUriComponentsBuilder.fromCurrentServletMapping().path("/api/groups").build().toUri()); + + return ResponseEntity.status(HttpStatus.NOT_FOUND).headers(headers) + .body(new ErrorResponse(String.valueOf(HttpStatus.NOT_FOUND.value()), + String.format("Unable to find group with resource id: [%s]", resourceId))); + } + return ResponseEntity.ok(g); + } + + @DeleteMapping("/{resourceId}") + @Transactional + public ResponseEntity delete(@PathVariable String resourceId) { + Group g = groupService.find(resourceId); + + if (g == null) { + HttpHeaders headers = new HttpHeaders(); + headers.setLocation(ServletUriComponentsBuilder.fromCurrentServletMapping().path("/api/groups").build().toUri()); + + return ResponseEntity.status(HttpStatus.NOT_FOUND).headers(headers) + .body(new ErrorResponse(String.valueOf(HttpStatus.NOT_FOUND.value()), + String.format("Unable to find group with resource id: [%s]", resourceId))); + } + try { + groupService.deleteDefinition(g); + } + catch (Exception e) { + HttpHeaders headers = new HttpHeaders(); + headers.setLocation(ServletUriComponentsBuilder.fromCurrentServletMapping().path("/api/groups").build().toUri()); + + return ResponseEntity.status(HttpStatus.CONFLICT).headers(headers) + .body(new ErrorResponse(String.valueOf(HttpStatus.CONFLICT.value()), String.format( + "Unable to delete group with resource id: [%s] - remove all users from group", + resourceId))); + } + return ResponseEntity.noContent().build(); + } +} diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/Group.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/Group.java new file mode 100644 index 000000000..92d836b59 --- /dev/null +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/Group.java @@ -0,0 +1,26 @@ +package edu.internet2.tier.shibboleth.admin.ui.domain; + +import java.util.UUID; + +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.Id; + +import org.hibernate.envers.Audited; + +import lombok.Data; + +@Entity(name = "user_groups") +@Audited +@Data +public class Group { + @Column(name = "group_description", nullable = true) + String description; + + @Column(nullable = false) + String name; + + @Id + @Column(name = "resource_id", nullable = false) + String resourceId = UUID.randomUUID().toString(); +} diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/repository/GroupRepository.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/repository/GroupRepository.java new file mode 100644 index 000000000..231e00348 --- /dev/null +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/repository/GroupRepository.java @@ -0,0 +1,16 @@ +package edu.internet2.tier.shibboleth.admin.ui.repository; + +import java.util.List; + +import org.springframework.data.jpa.repository.JpaRepository; + +import edu.internet2.tier.shibboleth.admin.ui.domain.Group; + +public interface GroupRepository extends JpaRepository { + List findAll(); + + Group findByResourceId(String id); + + @SuppressWarnings("unchecked") + Group save(Group group); +} diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/GroupServiceImpl.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/GroupServiceImpl.java new file mode 100644 index 000000000..03d4085ce --- /dev/null +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/GroupServiceImpl.java @@ -0,0 +1,36 @@ +package edu.internet2.tier.shibboleth.admin.ui.service; + +import java.util.List; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Service; + +import edu.internet2.tier.shibboleth.admin.ui.domain.Group; +import edu.internet2.tier.shibboleth.admin.ui.repository.GroupRepository; + +@Service +public class GroupServiceImpl implements IGroupService { + @Autowired + private GroupRepository repo; + + @Override + public Group createOrUpdateGroup(Group group) { + return repo.save(group); + } + + @Override + public void deleteDefinition(Group group) { + repo.delete(group); + } + + @Override + public Group find(String resourceId) { + return repo.findByResourceId(resourceId); + } + + @Override + public List findAll() { + return repo.findAll(); + } + +} diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/IGroupService.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/IGroupService.java new file mode 100644 index 000000000..1c5f92e93 --- /dev/null +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/IGroupService.java @@ -0,0 +1,17 @@ +package edu.internet2.tier.shibboleth.admin.ui.service; + +import java.util.List; + +import edu.internet2.tier.shibboleth.admin.ui.domain.Group; + +public interface IGroupService { + + Group createOrUpdateGroup(Group g); + + void deleteDefinition(Group g); + + Group find(String resourceId); + + List findAll(); + +} diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/repository/GroupRepositoryTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/repository/GroupRepositoryTests.groovy new file mode 100644 index 000000000..e7f59b23c --- /dev/null +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/repository/GroupRepositoryTests.groovy @@ -0,0 +1,144 @@ +package edu.internet2.tier.shibboleth.admin.ui.repository + +import javax.persistence.EntityManager + +import org.springframework.beans.factory.annotation.Autowired +import org.springframework.boot.autoconfigure.domain.EntityScan +import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest +import org.springframework.data.jpa.repository.config.EnableJpaRepositories +import org.springframework.test.context.ContextConfiguration + +import edu.internet2.tier.shibboleth.admin.ui.configuration.InternationalizationConfiguration +import edu.internet2.tier.shibboleth.admin.ui.domain.CustomEntityAttributeDefinition +import edu.internet2.tier.shibboleth.admin.ui.domain.Group +import spock.lang.Specification + +/** + * Tests to validate the repo and model for groups + * @author chasegawa + */ +@DataJpaTest +@ContextConfiguration(classes=[InternationalizationConfiguration]) +@EnableJpaRepositories(basePackages = ["edu.internet2.tier.shibboleth.admin.ui"]) +@EntityScan("edu.internet2.tier.shibboleth.admin.ui") +class GroupRepositoryTests extends Specification { + @Autowired + GroupRepository repo + + @Autowired + EntityManager entityManager + + def "simple create test"() { + given: + def group = new Group().with { + it.name = "group-name" + it.description = "some description" + it + } + + // Confirm empty state + when: + def groups = repo.findAll() + + then: + groups.size() == 0 + + // save check + when: + repo.save(group) + entityManager.flush() + entityManager.clear() + + then: + // save check + def gList = repo.findAll() + gList.size() == 1 + def groupFromDb = gList.get(0).asType(Group) + groupFromDb.equals(group) == true + + // fetch checks + repo.findByResourceId("not an id") == null + repo.findByResourceId(groupFromDb.resourceId).equals(group) + } + + def "expected error"() { + given: + def group = new Group().with { + it.description = "some description" + it + } + + // Confirm empty state + when: + def gList = repo.findAll() + + then: + gList.size() == 0 + + // save check + when: + repo.save(group) + entityManager.flush() + entityManager.clear() + + then: + // Missing non-nullable field (name) should thrown error + final def exception = thrown(javax.persistence.PersistenceException) + } + + def "basic CRUD operations validated"() { + given: + def group = new Group().with { + it.name = "group-name" + it.description = "some description" + it + } + + // Confirm empty state + when: + def groups = repo.findAll() + + then: + groups.size() == 0 + + // save check + when: + repo.save(group) + entityManager.flush() + entityManager.clear() + + then: + // save check + def gList = repo.findAll() + gList.size() == 1 + def groupFromDb = gList.get(0).asType(Group) + groupFromDb.equals(group) == true + + // update check + groupFromDb.with { + it.description = "some new text that wasn't there before" + } + groupFromDb.equals(group) == false + + when: + repo.save(groupFromDb) + entityManager.flush() + entityManager.clear() + + then: + def gList2 = repo.findAll() + gList2.size() == 1 + def groupFromDb2 = gList2.get(0).asType(Group) + groupFromDb2.equals(group) == false + groupFromDb2.equals(groupFromDb) == true + + // delete tests + when: + repo.delete(groupFromDb2) + entityManager.flush() + entityManager.clear() + + then: + repo.findAll().size() == 0 + } +} \ No newline at end of file From 58cf9878e645df9f79a773d419b587535416133a Mon Sep 17 00:00:00 2001 From: chasegawa Date: Wed, 23 Jun 2021 16:57:57 -0700 Subject: [PATCH 6/9] SHIBUI-1848 Moved user groups to security package --- .../ui/{ => security}/controller/GroupController.java | 7 ++++--- .../admin/ui/{domain => security/model}/Group.java | 2 +- .../ui/{ => security}/repository/GroupRepository.java | 4 ++-- .../admin/ui/{ => security}/service/GroupServiceImpl.java | 6 +++--- .../admin/ui/{ => security}/service/IGroupService.java | 4 ++-- .../{ => security}/repository/GroupRepositoryTests.groovy | 5 ++--- 6 files changed, 14 insertions(+), 14 deletions(-) rename backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/{ => security}/controller/GroupController.java (94%) rename backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/{domain => security/model}/Group.java (88%) rename backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/{ => security}/repository/GroupRepository.java (69%) rename backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/{ => security}/service/GroupServiceImpl.java (75%) rename backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/{ => security}/service/IGroupService.java (60%) rename backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/{ => security}/repository/GroupRepositoryTests.groovy (94%) diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/GroupController.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupController.java similarity index 94% rename from backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/GroupController.java rename to backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupController.java index bb54d113b..71e9ccf33 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/GroupController.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupController.java @@ -1,4 +1,4 @@ -package edu.internet2.tier.shibboleth.admin.ui.controller; +package edu.internet2.tier.shibboleth.admin.ui.security.controller; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpHeaders; @@ -15,8 +15,9 @@ import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.servlet.support.ServletUriComponentsBuilder; -import edu.internet2.tier.shibboleth.admin.ui.domain.Group; -import edu.internet2.tier.shibboleth.admin.ui.service.IGroupService; +import edu.internet2.tier.shibboleth.admin.ui.controller.ErrorResponse; +import edu.internet2.tier.shibboleth.admin.ui.security.model.Group; +import edu.internet2.tier.shibboleth.admin.ui.security.service.IGroupService; @Controller @RequestMapping(value = "/api/groups") diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/Group.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/Group.java similarity index 88% rename from backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/Group.java rename to backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/Group.java index 92d836b59..b91bcaf8e 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/Group.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/Group.java @@ -1,4 +1,4 @@ -package edu.internet2.tier.shibboleth.admin.ui.domain; +package edu.internet2.tier.shibboleth.admin.ui.security.model; import java.util.UUID; diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/repository/GroupRepository.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/repository/GroupRepository.java similarity index 69% rename from backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/repository/GroupRepository.java rename to backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/repository/GroupRepository.java index 231e00348..8781b69f4 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/repository/GroupRepository.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/repository/GroupRepository.java @@ -1,10 +1,10 @@ -package edu.internet2.tier.shibboleth.admin.ui.repository; +package edu.internet2.tier.shibboleth.admin.ui.security.repository; import java.util.List; import org.springframework.data.jpa.repository.JpaRepository; -import edu.internet2.tier.shibboleth.admin.ui.domain.Group; +import edu.internet2.tier.shibboleth.admin.ui.security.model.Group; public interface GroupRepository extends JpaRepository { List findAll(); diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/GroupServiceImpl.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/GroupServiceImpl.java similarity index 75% rename from backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/GroupServiceImpl.java rename to backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/GroupServiceImpl.java index 03d4085ce..4f6f1a762 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/GroupServiceImpl.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/GroupServiceImpl.java @@ -1,12 +1,12 @@ -package edu.internet2.tier.shibboleth.admin.ui.service; +package edu.internet2.tier.shibboleth.admin.ui.security.service; import java.util.List; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; -import edu.internet2.tier.shibboleth.admin.ui.domain.Group; -import edu.internet2.tier.shibboleth.admin.ui.repository.GroupRepository; +import edu.internet2.tier.shibboleth.admin.ui.security.model.Group; +import edu.internet2.tier.shibboleth.admin.ui.security.repository.GroupRepository; @Service public class GroupServiceImpl implements IGroupService { diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/IGroupService.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/IGroupService.java similarity index 60% rename from backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/IGroupService.java rename to backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/IGroupService.java index 1c5f92e93..1e040a9d1 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/IGroupService.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/IGroupService.java @@ -1,8 +1,8 @@ -package edu.internet2.tier.shibboleth.admin.ui.service; +package edu.internet2.tier.shibboleth.admin.ui.security.service; import java.util.List; -import edu.internet2.tier.shibboleth.admin.ui.domain.Group; +import edu.internet2.tier.shibboleth.admin.ui.security.model.Group; public interface IGroupService { diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/repository/GroupRepositoryTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/repository/GroupRepositoryTests.groovy similarity index 94% rename from backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/repository/GroupRepositoryTests.groovy rename to backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/repository/GroupRepositoryTests.groovy index e7f59b23c..fe0657403 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/repository/GroupRepositoryTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/repository/GroupRepositoryTests.groovy @@ -1,4 +1,4 @@ -package edu.internet2.tier.shibboleth.admin.ui.repository +package edu.internet2.tier.shibboleth.admin.ui.security.repository import javax.persistence.EntityManager @@ -9,8 +9,7 @@ import org.springframework.data.jpa.repository.config.EnableJpaRepositories import org.springframework.test.context.ContextConfiguration import edu.internet2.tier.shibboleth.admin.ui.configuration.InternationalizationConfiguration -import edu.internet2.tier.shibboleth.admin.ui.domain.CustomEntityAttributeDefinition -import edu.internet2.tier.shibboleth.admin.ui.domain.Group +import edu.internet2.tier.shibboleth.admin.ui.security.model.Group import spock.lang.Specification /** From 47e21b62fe48940732188b9c0613668db1517a98 Mon Sep 17 00:00:00 2001 From: chasegawa Date: Thu, 24 Jun 2021 16:28:01 -0700 Subject: [PATCH 7/9] SHIBUI-1848 Corrected groups controller and added testing --- .../admin/ui/configuration/DevConfig.groovy | 27 ++- .../security/controller/GroupController.java | 20 +- .../admin/ui/security/model/Group.java | 2 +- ...pRepository.java => GroupsRepository.java} | 2 +- .../ui/security/service/GroupServiceImpl.java | 4 +- .../GroupsControllerIntegrationTests.groovy | 179 ++++++++++++++++++ ...ts.groovy => GroupsRepositoryTests.groovy} | 4 +- 7 files changed, 221 insertions(+), 17 deletions(-) rename backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/repository/{GroupRepository.java => GroupsRepository.java} (83%) create mode 100644 backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupsControllerIntegrationTests.groovy rename backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/repository/{GroupRepositoryTests.groovy => GroupsRepositoryTests.groovy} (98%) diff --git a/backend/src/main/groovy/edu/internet2/tier/shibboleth/admin/ui/configuration/DevConfig.groovy b/backend/src/main/groovy/edu/internet2/tier/shibboleth/admin/ui/configuration/DevConfig.groovy index 9e97395db..6080b2f20 100644 --- a/backend/src/main/groovy/edu/internet2/tier/shibboleth/admin/ui/configuration/DevConfig.groovy +++ b/backend/src/main/groovy/edu/internet2/tier/shibboleth/admin/ui/configuration/DevConfig.groovy @@ -12,8 +12,10 @@ import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.ReloadableMetadat import edu.internet2.tier.shibboleth.admin.ui.opensaml.OpenSamlObjects import edu.internet2.tier.shibboleth.admin.ui.repository.EntityDescriptorRepository import edu.internet2.tier.shibboleth.admin.ui.repository.MetadataResolverRepository +import edu.internet2.tier.shibboleth.admin.ui.security.model.Group import edu.internet2.tier.shibboleth.admin.ui.security.model.Role import edu.internet2.tier.shibboleth.admin.ui.security.model.User +import edu.internet2.tier.shibboleth.admin.ui.security.repository.GroupsRepository import edu.internet2.tier.shibboleth.admin.ui.security.repository.RoleRepository import edu.internet2.tier.shibboleth.admin.ui.security.repository.UserRepository import edu.internet2.tier.shibboleth.admin.util.ModelRepresentationConversions @@ -29,6 +31,7 @@ import javax.annotation.PostConstruct @Profile('dev') class DevConfig { private final UserRepository adminUserRepository + private final GroupsRepository groupsRepository private final RoleRepository roleRepository private final MetadataResolverRepository metadataResolverRepository @@ -37,6 +40,7 @@ class DevConfig { private final OpenSamlObjects openSamlObjects DevConfig(UserRepository adminUserRepository, + GroupsRepository groupsRepository, MetadataResolverRepository metadataResolverRepository, RoleRepository roleRepository, EntityDescriptorRepository entityDescriptorRepository, @@ -47,11 +51,32 @@ class DevConfig { this.roleRepository = roleRepository this.entityDescriptorRepository = entityDescriptorRepository this.openSamlObjects = openSamlObjects + this.groupsRepository = groupsRepository } @Transactional @PostConstruct - void createDevUsers() { + void createDevUsersAndGroups() { + if (groupsRepository.count() == 0) { + def groups = [ + new Group().with { + it.name = "A1" + it.description = "AAA Group" + it.resourceId = "AAA" + it + }, + new Group().with { + it.name = "B1" + it.description = "BBB Group" + it.resourceId = "BBB" + it + }] + groups.each { + groupsRepository.save(it) + } + } + groupsRepository.flush() + if (roleRepository.count() == 0) { def roles = [new Role().with { name = 'ROLE_ADMIN' diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupController.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupController.java index 71e9ccf33..54ba0f863 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupController.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupController.java @@ -20,7 +20,7 @@ import edu.internet2.tier.shibboleth.admin.ui.security.service.IGroupService; @Controller -@RequestMapping(value = "/api/groups") +@RequestMapping(value = "/api/admin/groups") public class GroupController { @Autowired private IGroupService groupService; @@ -29,11 +29,11 @@ public class GroupController { @Transactional public ResponseEntity create(@RequestBody Group group) { // If already defined, we can't create a new one, nor will this call update the definition - Group g = groupService.find(group.getResourceId()); + Group foundGroup = groupService.find(group.getResourceId()); - if (g != null) { + if (foundGroup != null) { HttpHeaders headers = new HttpHeaders(); - headers.setLocation(ServletUriComponentsBuilder.fromCurrentServletMapping().path("/api/groups").build().toUri()); + headers.setLocation(ServletUriComponentsBuilder.fromCurrentServletMapping().path("/api/admin/groups").build().toUri()); return ResponseEntity.status(HttpStatus.METHOD_NOT_ALLOWED).headers(headers) .body(new ErrorResponse(String.valueOf(HttpStatus.METHOD_NOT_ALLOWED.value()), @@ -41,7 +41,7 @@ public ResponseEntity create(@RequestBody Group group) { group.getResourceId(), group.getName()))); } - Group result = groupService.createOrUpdateGroup(g); + Group result = groupService.createOrUpdateGroup(group); return ResponseEntity.status(HttpStatus.CREATED).body(result); } @@ -52,7 +52,7 @@ public ResponseEntity update(@RequestBody Group group) { if (g == null) { HttpHeaders headers = new HttpHeaders(); - headers.setLocation(ServletUriComponentsBuilder.fromCurrentServletMapping().path("/api/groups").build().toUri()); + headers.setLocation(ServletUriComponentsBuilder.fromCurrentServletMapping().path("/api/admin/groups").build().toUri()); return ResponseEntity.status(HttpStatus.NOT_FOUND).headers(headers) .body(new ErrorResponse(String.valueOf(HttpStatus.NOT_FOUND.value()), @@ -60,7 +60,7 @@ public ResponseEntity update(@RequestBody Group group) { group.getResourceId(), group.getName()))); } - Group result = groupService.createOrUpdateGroup(g); + Group result = groupService.createOrUpdateGroup(group); return ResponseEntity.ok(result); } @@ -77,7 +77,7 @@ public ResponseEntity getOne(@PathVariable String resourceId) { if (g == null) { HttpHeaders headers = new HttpHeaders(); - headers.setLocation(ServletUriComponentsBuilder.fromCurrentServletMapping().path("/api/groups").build().toUri()); + headers.setLocation(ServletUriComponentsBuilder.fromCurrentServletMapping().path("/api/admin/groups").build().toUri()); return ResponseEntity.status(HttpStatus.NOT_FOUND).headers(headers) .body(new ErrorResponse(String.valueOf(HttpStatus.NOT_FOUND.value()), @@ -93,7 +93,7 @@ public ResponseEntity delete(@PathVariable String resourceId) { if (g == null) { HttpHeaders headers = new HttpHeaders(); - headers.setLocation(ServletUriComponentsBuilder.fromCurrentServletMapping().path("/api/groups").build().toUri()); + headers.setLocation(ServletUriComponentsBuilder.fromCurrentServletMapping().path("/api/admin/groups").build().toUri()); return ResponseEntity.status(HttpStatus.NOT_FOUND).headers(headers) .body(new ErrorResponse(String.valueOf(HttpStatus.NOT_FOUND.value()), @@ -104,7 +104,7 @@ public ResponseEntity delete(@PathVariable String resourceId) { } catch (Exception e) { HttpHeaders headers = new HttpHeaders(); - headers.setLocation(ServletUriComponentsBuilder.fromCurrentServletMapping().path("/api/groups").build().toUri()); + headers.setLocation(ServletUriComponentsBuilder.fromCurrentServletMapping().path("/api/admin/groups").build().toUri()); return ResponseEntity.status(HttpStatus.CONFLICT).headers(headers) .body(new ErrorResponse(String.valueOf(HttpStatus.CONFLICT.value()), String.format( diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/Group.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/Group.java index b91bcaf8e..65880b3b8 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/Group.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/Group.java @@ -21,6 +21,6 @@ public class Group { String name; @Id - @Column(name = "resource_id", nullable = false) + @Column(name = "resource_id") String resourceId = UUID.randomUUID().toString(); } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/repository/GroupRepository.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/repository/GroupsRepository.java similarity index 83% rename from backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/repository/GroupRepository.java rename to backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/repository/GroupsRepository.java index 8781b69f4..9576184e4 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/repository/GroupRepository.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/repository/GroupsRepository.java @@ -6,7 +6,7 @@ import edu.internet2.tier.shibboleth.admin.ui.security.model.Group; -public interface GroupRepository extends JpaRepository { +public interface GroupsRepository extends JpaRepository { List findAll(); Group findByResourceId(String id); diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/GroupServiceImpl.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/GroupServiceImpl.java index 4f6f1a762..f4c3a1344 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/GroupServiceImpl.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/GroupServiceImpl.java @@ -6,12 +6,12 @@ import org.springframework.stereotype.Service; import edu.internet2.tier.shibboleth.admin.ui.security.model.Group; -import edu.internet2.tier.shibboleth.admin.ui.security.repository.GroupRepository; +import edu.internet2.tier.shibboleth.admin.ui.security.repository.GroupsRepository; @Service public class GroupServiceImpl implements IGroupService { @Autowired - private GroupRepository repo; + private GroupsRepository repo; @Override public Group createOrUpdateGroup(Group group) { diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupsControllerIntegrationTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupsControllerIntegrationTests.groovy new file mode 100644 index 000000000..ed2030e4d --- /dev/null +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupsControllerIntegrationTests.groovy @@ -0,0 +1,179 @@ +package edu.internet2.tier.shibboleth.admin.ui.security.controller + + +import groovy.json.JsonOutput +import org.springframework.beans.factory.annotation.Autowired +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc +import org.springframework.boot.test.context.SpringBootTest +import org.springframework.http.MediaType +import org.springframework.security.test.context.support.WithMockUser +import org.springframework.test.annotation.Rollback +import org.springframework.test.context.ActiveProfiles +import org.springframework.test.web.servlet.MockMvc +import org.springframework.test.web.servlet.result.MockMvcResultHandlers +import org.springframework.transaction.annotation.Transactional + +import spock.lang.Ignore +import spock.lang.Specification + +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.patch +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status + +@SpringBootTest +@AutoConfigureMockMvc +@ActiveProfiles(["no-auth", "dev"]) +@Transactional +class GroupsControllerIntegrationTests extends Specification { + @Autowired + private MockMvc mockMvc + + static RESOURCE_URI = '/api/admin/groups' + + @Rollback + @WithMockUser(value = "admin", roles = ["ADMIN"]) + def 'POST new group persists properly'() { + given: + def newGroup = [name: 'Foo', + description: 'Bar', + resourceId: 'FooBar'] + + def expectedJson = """ + { + "name":"Foo", + "description":"Bar", + "resourceId":"FooBar" + } +""" + + when: + def result = mockMvc.perform(post(RESOURCE_URI) + .contentType(MediaType.APPLICATION_JSON) + .content(JsonOutput.toJson(newGroup)) + .accept(MediaType.APPLICATION_JSON)) + + then: + result.andExpect(status().isCreated()) + .andExpect(content().contentType(MediaType.APPLICATION_JSON)) + .andExpect(content().json(expectedJson, false)) + + when: 'Try to create with an existing resource id' + result = mockMvc.perform(post(RESOURCE_URI) + .contentType(MediaType.APPLICATION_JSON) + .content(JsonOutput.toJson(newGroup)) + .accept(MediaType.APPLICATION_JSON)) + + then: 'Expecting method not allowed' + result.andExpect(status().isMethodNotAllowed()) + } + + @Rollback + @WithMockUser(value = "admin", roles = ["ADMIN"]) + def 'PUT (update) existing group persists properly'() { + given: + def group = [name: 'NOT AAA', + description: 'updated AAA', + resourceId: 'AAA'] + + def expectedJson = """ + { + "name":"NOT AAA", + "description":"updated AAA", + "resourceId":"AAA" + } +""" + when: + def result = mockMvc.perform(put(RESOURCE_URI) + .contentType(MediaType.APPLICATION_JSON) + .content(JsonOutput.toJson(group)) + .accept(MediaType.APPLICATION_JSON)) + + then: + result.andExpect(status().isOk()) + .andExpect(content().contentType(MediaType.APPLICATION_JSON)) + .andExpect(content().json(expectedJson, false)) + + when: 'Try to update with a non-existing resource id' + def newGroup = [name: 'XXXXX', + description: 'should not work', + resourceId: 'XXXX'] + def notFoundresult = mockMvc.perform(put(RESOURCE_URI) + .contentType(MediaType.APPLICATION_JSON) + .content(JsonOutput.toJson(newGroup)) + .accept(MediaType.APPLICATION_JSON)) + + then: 'Expecting nothing happened because the object was not found' + notFoundresult.andExpect(status().isNotFound()) + } + + @WithMockUser(value = "admin", roles = ["ADMIN"]) + def 'GET checks for groups (when there are existing groups)'() { + given: + def expectedJson = """ +[ + { + "name":"A1", + "description":"AAA Group", + "resourceId":"AAA" + }, + { + "name":"B1", + "description":"BBB Group", + "resourceId":"BBB" + } +]""" + when: 'GET request is made for ALL groups in the system, and system has groups in it' + def result = mockMvc.perform(get(RESOURCE_URI)) + + then: 'Request completed with HTTP 200 and returned a list of users' + result.andExpect(status().isOk()) + .andExpect(content().contentType(MediaType.APPLICATION_JSON)) + .andExpect(content().json(expectedJson, false)) + + when: 'GET request for a single specific group in a system that has groups' + def singleGroupRequest = mockMvc.perform(get("$RESOURCE_URI/BBB")) + + then: 'GET request for a single specific group completed with HTTP 200' + singleGroupRequest.andExpect(status().isOk()) + + when: 'GET request for a single non-existent group in a system that has groups' + def nonexistentGroupRequest = mockMvc.perform(get("$RESOURCE_URI/CCC")) + + then: 'The group not found' + nonexistentGroupRequest.andExpect(status().isNotFound()) + } + + @Rollback + @WithMockUser(value = "admin", roles = ["ADMIN"]) + def 'DELETE ONE existing group'() { + when: 'GET request for a single specific group in a system that has groups' + def result = mockMvc.perform(get("$RESOURCE_URI/BBB")) + + then: 'GET request for a single specific group completed with HTTP 200' + result.andExpect(status().isOk()) + + when: 'DELETE request is made' + result = mockMvc.perform(delete("$RESOURCE_URI/BBB")) + + then: 'DELETE was successful' + result.andExpect(status().isNoContent()) + + when: 'GET request for a single specific group just deleted' + result = mockMvc.perform(get("$RESOURCE_URI/BBB")) + + then: 'The group not found' + result.andExpect(status().isNotFound()) + + when: 'DELETE request for a single specific group that does not exist' + result = mockMvc.perform(delete("$RESOURCE_URI/CCCC")) + + then: 'The group not found' + result.andExpect(status().isNotFound()) + + //ADD conflict test when the group has users + } +} diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/repository/GroupRepositoryTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/repository/GroupsRepositoryTests.groovy similarity index 98% rename from backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/repository/GroupRepositoryTests.groovy rename to backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/repository/GroupsRepositoryTests.groovy index fe0657403..20fcc4f8a 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/repository/GroupRepositoryTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/repository/GroupsRepositoryTests.groovy @@ -20,9 +20,9 @@ import spock.lang.Specification @ContextConfiguration(classes=[InternationalizationConfiguration]) @EnableJpaRepositories(basePackages = ["edu.internet2.tier.shibboleth.admin.ui"]) @EntityScan("edu.internet2.tier.shibboleth.admin.ui") -class GroupRepositoryTests extends Specification { +class GroupsRepositoryTests extends Specification { @Autowired - GroupRepository repo + GroupsRepository repo @Autowired EntityManager entityManager From b18290c849ce46b494a90fa2c5c22dbcc7b4be8c Mon Sep 17 00:00:00 2001 From: chasegawa Date: Thu, 24 Jun 2021 21:35:08 -0700 Subject: [PATCH 8/9] SHIBUI-1848 finished adjustments to groups controller. Added group field to user --- .../security/controller/GroupController.java | 8 +- .../admin/ui/security/model/Group.java | 13 ++- .../admin/ui/security/model/User.java | 25 +++-- .../GroupsControllerIntegrationTests.groovy | 93 ++++++++++++++----- 4 files changed, 102 insertions(+), 37 deletions(-) diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupController.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupController.java index 54ba0f863..da6d5edae 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupController.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupController.java @@ -99,18 +99,16 @@ public ResponseEntity delete(@PathVariable String resourceId) { .body(new ErrorResponse(String.valueOf(HttpStatus.NOT_FOUND.value()), String.format("Unable to find group with resource id: [%s]", resourceId))); } - try { - groupService.deleteDefinition(g); - } - catch (Exception e) { + if (!g.getUsers().isEmpty()) { HttpHeaders headers = new HttpHeaders(); headers.setLocation(ServletUriComponentsBuilder.fromCurrentServletMapping().path("/api/admin/groups").build().toUri()); return ResponseEntity.status(HttpStatus.CONFLICT).headers(headers) .body(new ErrorResponse(String.valueOf(HttpStatus.CONFLICT.value()), String.format( - "Unable to delete group with resource id: [%s] - remove all users from group", + "Unable to delete group with resource id: [%s] - remove all users from group first", resourceId))); } + groupService.deleteDefinition(g); return ResponseEntity.noContent().build(); } } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/Group.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/Group.java index 65880b3b8..db903e988 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/Group.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/Group.java @@ -1,17 +1,21 @@ package edu.internet2.tier.shibboleth.admin.ui.security.model; +import java.util.Set; import java.util.UUID; +import javax.persistence.CascadeType; import javax.persistence.Column; import javax.persistence.Entity; +import javax.persistence.FetchType; import javax.persistence.Id; +import javax.persistence.OneToMany; -import org.hibernate.envers.Audited; +import com.fasterxml.jackson.annotation.JsonIgnore; import lombok.Data; +import lombok.EqualsAndHashCode; @Entity(name = "user_groups") -@Audited @Data public class Group { @Column(name = "group_description", nullable = true) @@ -23,4 +27,9 @@ public class Group { @Id @Column(name = "resource_id") String resourceId = UUID.randomUUID().toString(); + + @OneToMany(mappedBy = "group", cascade = CascadeType.ALL, fetch = FetchType.EAGER) + @JsonIgnore + @EqualsAndHashCode.Exclude + Set users; } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/User.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/User.java index c30c4ceff..df4a96e13 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/User.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/User.java @@ -10,12 +10,15 @@ import lombok.ToString; import org.apache.commons.lang.StringUtils; +import javax.persistence.CascadeType; import javax.persistence.Column; import javax.persistence.Entity; import javax.persistence.FetchType; import javax.persistence.JoinColumn; import javax.persistence.JoinTable; import javax.persistence.ManyToMany; +import javax.persistence.ManyToOne; +import javax.persistence.OneToOne; import javax.persistence.Table; import javax.persistence.Transient; import java.util.HashSet; @@ -30,23 +33,25 @@ @NoArgsConstructor @Getter @Setter -@EqualsAndHashCode(callSuper = true, exclude = "roles") +@EqualsAndHashCode(callSuper = true) @ToString(exclude = "roles") @Table(name = "USERS") public class User extends AbstractAuditable { - @Column(nullable = false, unique = true) - private String username; - - @JsonProperty(access = JsonProperty.Access.WRITE_ONLY) - @Column(nullable = false) - private String password; + private String emailAddress; private String firstName; + @ManyToOne + @JoinColumn(name = "resource_id") + @EqualsAndHashCode.Exclude + private Group group; + private String lastName; - private String emailAddress; + @JsonProperty(access = JsonProperty.Access.WRITE_ONLY) + @Column(nullable = false) + private String password; @Transient private String role; @@ -54,8 +59,12 @@ public class User extends AbstractAuditable { @JsonIgnore @ManyToMany(fetch = FetchType.EAGER) @JoinTable(name = "user_role", joinColumns = @JoinColumn(name = "user_id"), inverseJoinColumns = @JoinColumn(name = "role_id")) + @EqualsAndHashCode.Exclude private Set roles = new HashSet<>(); + @Column(nullable = false, unique = true) + private String username; + public String getRole() { if (StringUtils.isBlank(this.role)) { Set roles = this.getRoles(); diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupsControllerIntegrationTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupsControllerIntegrationTests.groovy index ed2030e4d..4b83af244 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupsControllerIntegrationTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupsControllerIntegrationTests.groovy @@ -33,6 +33,7 @@ class GroupsControllerIntegrationTests extends Specification { private MockMvc mockMvc static RESOURCE_URI = '/api/admin/groups' + static USERS_RESOURCE_URI = '/api/admin/users' @Rollback @WithMockUser(value = "admin", roles = ["ADMIN"]) @@ -147,33 +148,81 @@ class GroupsControllerIntegrationTests extends Specification { nonexistentGroupRequest.andExpect(status().isNotFound()) } +// @Rollback +// @WithMockUser(value = "admin", roles = ["ADMIN"]) +// def 'DELETE ONE existing group'() { +// when: 'GET request for a single specific group in a system that has groups' +// def result = mockMvc.perform(get("$RESOURCE_URI/BBB")) +// +// then: 'GET request for a single specific group completed with HTTP 200' +// result.andExpect(status().isOk()) +// +// when: 'DELETE request is made' +// result = mockMvc.perform(delete("$RESOURCE_URI/BBB")) +// +// then: 'DELETE was successful' +// result.andExpect(status().isNoContent()) +// +// when: 'GET request for a single specific group just deleted' +// result = mockMvc.perform(get("$RESOURCE_URI/BBB")) +// +// then: 'The group not found' +// result.andExpect(status().isNotFound()) +// +// when: 'DELETE request for a single specific group that does not exist' +// result = mockMvc.perform(delete("$RESOURCE_URI/CCCC")) +// +// then: 'The group not found' +// result.andExpect(status().isNotFound()) +// } + @Rollback @WithMockUser(value = "admin", roles = ["ADMIN"]) - def 'DELETE ONE existing group'() { - when: 'GET request for a single specific group in a system that has groups' - def result = mockMvc.perform(get("$RESOURCE_URI/BBB")) + def 'DELETE performs correctly when group attached to a user'() { + given: + def group = [name: 'A1', + description: 'AAA Group', + resourceId: 'AAA'] + def newUser = [firstName: 'Foo', + lastName: 'Bar', + username: 'FooBar', + password: 'somepass', + emailAddress: 'foo@institution.edu', + role: 'ROLE_USER', + group: group] - then: 'GET request for a single specific group completed with HTTP 200' + when: + def result = mockMvc.perform(post(USERS_RESOURCE_URI) + .contentType(MediaType.APPLICATION_JSON) + .content(JsonOutput.toJson(newUser)) + .accept(MediaType.APPLICATION_JSON)) + + then: result.andExpect(status().isOk()) - - when: 'DELETE request is made' - result = mockMvc.perform(delete("$RESOURCE_URI/BBB")) - - then: 'DELETE was successful' - result.andExpect(status().isNoContent()) - - when: 'GET request for a single specific group just deleted' - result = mockMvc.perform(get("$RESOURCE_URI/BBB")) - - then: 'The group not found' - result.andExpect(status().isNotFound()) + + when: + def userresult = mockMvc.perform(get("$USERS_RESOURCE_URI/$newUser.username")) + def expectedJson = """ +{ + "modifiedBy" : admin, + "firstName" : "Foo", + "emailAddress" : "foo@institution.edu", + "role" : "ROLE_USER", + "username" : "FooBar", + "createdBy" : admin, + "lastName" : "Bar", + "group" : {"description":"AAA Group","name":"A1","resourceId":"AAA"} +}""" + then: 'Request completed with HTTP 200 and returned one user' + userresult.andExpect(status().isOk()) + .andExpect(content().contentType(MediaType.APPLICATION_JSON)) + .andExpect(content().json(expectedJson, false)) - when: 'DELETE request for a single specific group that does not exist' - result = mockMvc.perform(delete("$RESOURCE_URI/CCCC")) - - then: 'The group not found' - result.andExpect(status().isNotFound()) - //ADD conflict test when the group has users + when: 'DELETE request is made' + result = mockMvc.perform(delete("$RESOURCE_URI/$group.resourceId")) + + then: 'DELETE was not successful' + result.andExpect(status().isConflict()) } } From 3e98da8f0cc2ebe8e17cd5aa129bdfebdb0393fe Mon Sep 17 00:00:00 2001 From: chasegawa Date: Thu, 24 Jun 2021 21:59:46 -0700 Subject: [PATCH 9/9] SHIBUI-1848 Adjusting user test --- .../controller/GroupsControllerIntegrationTests.groovy | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupsControllerIntegrationTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupsControllerIntegrationTests.groovy index 4b83af244..3b514c065 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupsControllerIntegrationTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupsControllerIntegrationTests.groovy @@ -180,9 +180,7 @@ class GroupsControllerIntegrationTests extends Specification { @WithMockUser(value = "admin", roles = ["ADMIN"]) def 'DELETE performs correctly when group attached to a user'() { given: - def group = [name: 'A1', - description: 'AAA Group', - resourceId: 'AAA'] + def group = [resourceId: 'AAA'] def newUser = [firstName: 'Foo', lastName: 'Bar', username: 'FooBar', @@ -211,7 +209,7 @@ class GroupsControllerIntegrationTests extends Specification { "username" : "FooBar", "createdBy" : admin, "lastName" : "Bar", - "group" : {"description":"AAA Group","name":"A1","resourceId":"AAA"} + "group" : {"resourceId":"AAA"} }""" then: 'Request completed with HTTP 200 and returned one user' userresult.andExpect(status().isOk())