From db7370038e5b9b9aaeb70d5525049d7c110dc7c5 Mon Sep 17 00:00:00 2001 From: Ryan Mathis Date: Tue, 22 Jun 2021 10:21:40 -0700 Subject: [PATCH 01/61] initial commit for groups --- ui/public/assets/schema/groups/group.json | 15 ++++ ui/src/app/App.js | 4 +- ui/src/app/admin/Groups.js | 32 ++++++++ ui/src/app/admin/component/GroupForm.js | 69 ++++++++++++++++ ui/src/app/admin/container/EditGroup.js | 5 ++ ui/src/app/admin/container/GroupsList.js | 79 +++++++++++++++++++ ui/src/app/admin/container/NewGroup.js | 59 ++++++++++++++ ui/src/app/admin/hoc/GroupsProvider.js | 35 ++++++++ ui/src/app/admin/hooks.js | 16 ++++ .../components}/DeleteConfirmation.js | 0 .../components}/DeleteConfirmation.test.js | 0 ui/src/app/core/components/Header.js | 6 +- ui/src/app/form/Schema.js | 24 ++++++ .../filter/component/MetadataFilters.js | 2 +- ui/src/app/metadata/hooks/api.js | 2 +- .../metadata/view/MetadataAttributeList.js | 2 +- 16 files changed, 345 insertions(+), 5 deletions(-) create mode 100644 ui/public/assets/schema/groups/group.json create mode 100644 ui/src/app/admin/Groups.js create mode 100644 ui/src/app/admin/component/GroupForm.js create mode 100644 ui/src/app/admin/container/EditGroup.js create mode 100644 ui/src/app/admin/container/GroupsList.js create mode 100644 ui/src/app/admin/container/NewGroup.js create mode 100644 ui/src/app/admin/hoc/GroupsProvider.js create mode 100644 ui/src/app/admin/hooks.js rename ui/src/app/{metadata/component => core/components}/DeleteConfirmation.js (100%) rename ui/src/app/{metadata/component => core/components}/DeleteConfirmation.test.js (100%) create mode 100644 ui/src/app/form/Schema.js diff --git a/ui/public/assets/schema/groups/group.json b/ui/public/assets/schema/groups/group.json new file mode 100644 index 000000000..088d861f6 --- /dev/null +++ b/ui/public/assets/schema/groups/group.json @@ -0,0 +1,15 @@ +{ + "type": "object", + "required": [ + "name" + ], + "properties": { + "name": { + "title": "label.group-name", + "description": "tooltip.group-name", + "type": "string", + "minLength": 1, + "maxLength": 255 + } + } +} \ No newline at end of file diff --git a/ui/src/app/App.js b/ui/src/app/App.js index 097c0095d..58456350d 100644 --- a/ui/src/app/App.js +++ b/ui/src/app/App.js @@ -27,6 +27,7 @@ import { Filter } from './metadata/Filter'; import { Contention } from './metadata/contention/ContentionContext'; import { SessionModal } from './core/user/SessionModal'; import Button from 'react-bootstrap/Button'; +import { Groups } from './admin/Groups'; function App() { @@ -76,8 +77,9 @@ function App() { - + + diff --git a/ui/src/app/admin/Groups.js b/ui/src/app/admin/Groups.js new file mode 100644 index 000000000..ec69c4f61 --- /dev/null +++ b/ui/src/app/admin/Groups.js @@ -0,0 +1,32 @@ +import React from 'react'; +import { Switch, Route, useRouteMatch, Redirect } from 'react-router-dom'; +import { GroupsProvider } from './hoc/GroupsProvider'; +import { NewGroup } from './container/NewGroup'; +import { EditGroup } from './container/EditGroup'; +import { GroupsList } from './container/GroupsList'; + +export function Groups() { + + let { path } = useRouteMatch(); + + return ( + <> + + + + {(groups, onDelete) => + + } + + } /> + + + } /> + + + } /> + + + + ); +} \ No newline at end of file diff --git a/ui/src/app/admin/component/GroupForm.js b/ui/src/app/admin/component/GroupForm.js new file mode 100644 index 000000000..85a08d587 --- /dev/null +++ b/ui/src/app/admin/component/GroupForm.js @@ -0,0 +1,69 @@ +import React from 'react'; +import Button from 'react-bootstrap/Button'; +import Form from '@rjsf/bootstrap-4'; +import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; +import { faSpinner, faSave } from '@fortawesome/free-solid-svg-icons'; +import Translate from '../../i18n/components/translate'; + +import { useGroupUiSchema } from '../hooks'; +import { fields, widgets } from '../../form/component'; +import { templates } from '../../form/component'; + +function ErrorListTemplate() { + return (<>); +} + +export function GroupForm ({schema}) { + + const [errors, setErrors] = React.useState([]); + const [loading, setLoading] = React.useState(false); + const [metadata, setMetadata] = React.useState({}); + + const save = () => { }; + const cancel = () => { }; + const onChange = () => { }; + + const uiSchema = useGroupUiSchema(); + + return (<> +
+
+ + + + +
+
+
+
+
onChange(form)} + schema={schema} + uiSchema={uiSchema} + FieldTemplate={templates.FieldTemplate} + ObjectFieldTemplate={templates.ObjectFieldTemplate} + ArrayFieldTemplate={templates.ArrayFieldTemplate} + fields={fields} + widgets={widgets} + liveValidate={true} + ErrorList={ErrorListTemplate}> + <> +
+
+
+
+ ) +} +/**/ \ No newline at end of file diff --git a/ui/src/app/admin/container/EditGroup.js b/ui/src/app/admin/container/EditGroup.js new file mode 100644 index 000000000..355a1d6e2 --- /dev/null +++ b/ui/src/app/admin/container/EditGroup.js @@ -0,0 +1,5 @@ +import React from 'react'; + +export function EditGroup () { + return (<>Edit Group); +} \ No newline at end of file diff --git a/ui/src/app/admin/container/GroupsList.js b/ui/src/app/admin/container/GroupsList.js new file mode 100644 index 000000000..023b86ff5 --- /dev/null +++ b/ui/src/app/admin/container/GroupsList.js @@ -0,0 +1,79 @@ +import React from 'react'; +import { faEdit, faPlusCircle, faTrash } from '@fortawesome/free-solid-svg-icons'; +import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; + +import Button from 'react-bootstrap/Button'; +import { Link } from 'react-router-dom'; + +import { Translate } from '../../i18n/components/translate'; +import { useTranslator } from '../../i18n/hooks'; + +import { DeleteConfirmation } from '../../core/components/DeleteConfirmation'; + +export function GroupsList({ groups, onDelete }) { + + const translator = useTranslator(); + + const remove = (id) => { + onDelete(id); + } + + return ( + + {(block) => +
+
+
+
+ + Groups Management + +
+
+
+ +   + Add new group + +
+
+ + + + + + + + + {groups?.length && groups.map((group, i) => + + + + + )} + +
+ Group Name + Actions
{group.name} + + + + Edit + + + +
+
+
+
+
+
+ } + + ); +} \ No newline at end of file diff --git a/ui/src/app/admin/container/NewGroup.js b/ui/src/app/admin/container/NewGroup.js new file mode 100644 index 000000000..ab5e7d3ee --- /dev/null +++ b/ui/src/app/admin/container/NewGroup.js @@ -0,0 +1,59 @@ +import React from 'react'; + +import { Prompt, useHistory } from 'react-router'; +import Translate from '../../i18n/components/translate'; +import { useGroups } from '../hooks'; +import { Schema } from '../../form/Schema'; + +import { GroupForm } from '../component/GroupForm'; + +export function NewGroup() { + const history = useHistory(); + + const { post, response, loading } = useGroups({}); + + const [blocking, setBlocking] = React.useState(false); + + async function save(metadata) { + await post(``, metadata); + if (response.ok) { + gotoDetail({ refresh: true }); + } + }; + + const cancel = () => { + gotoDetail(); + }; + + const gotoDetail = (state = null) => { + setBlocking(false); + history.push(`/groups`, state); + }; + + const [group, setGroup] = React.useState({}); + + return ( +
+ + `message.unsaved-editor` + } + /> +
+
+
+
+ Add a new group +
+
+
+
+ + {(schema) => } + +
+
+
+ ); +} \ No newline at end of file diff --git a/ui/src/app/admin/hoc/GroupsProvider.js b/ui/src/app/admin/hoc/GroupsProvider.js new file mode 100644 index 000000000..a9d47131b --- /dev/null +++ b/ui/src/app/admin/hoc/GroupsProvider.js @@ -0,0 +1,35 @@ +import React from 'react'; +import { useGroups } from '../hooks'; + +export function GroupsProvider({ children }) { + + const [groups, setGroups] = React.useState([ + { + name: 'foo' + } + ]); + + + const { get, del, response } = useGroups({ + cachePolicy: 'no-cache' + }); + + async function loadGroups() { + const list = await get(``); + if (response.ok) { + setGroups(list); + } + } + + async function removeGroup(id) { + await del(`/${id}`); + if (response.ok) { + loadGroups(); + } + } + + /*eslint-disable react-hooks/exhaustive-deps*/ + // React.useEffect(() => { loadGroups() }, []); + + return (<>{children(groups, removeGroup)}); +} \ No newline at end of file diff --git a/ui/src/app/admin/hooks.js b/ui/src/app/admin/hooks.js new file mode 100644 index 000000000..d071fb5e4 --- /dev/null +++ b/ui/src/app/admin/hooks.js @@ -0,0 +1,16 @@ +import useFetch from 'use-http'; +import API_BASE_PATH from '../App.constant'; + +export function useGroups () { + return useFetch(`${API_BASE_PATH}/groups`); +} + +export function useGroup() { + return useFetch(`${API_BASE_PATH}/group`); +} + +export function useGroupUiSchema () { + return { + + }; +} \ No newline at end of file diff --git a/ui/src/app/metadata/component/DeleteConfirmation.js b/ui/src/app/core/components/DeleteConfirmation.js similarity index 100% rename from ui/src/app/metadata/component/DeleteConfirmation.js rename to ui/src/app/core/components/DeleteConfirmation.js diff --git a/ui/src/app/metadata/component/DeleteConfirmation.test.js b/ui/src/app/core/components/DeleteConfirmation.test.js similarity index 100% rename from ui/src/app/metadata/component/DeleteConfirmation.test.js rename to ui/src/app/core/components/DeleteConfirmation.test.js diff --git a/ui/src/app/core/components/Header.js b/ui/src/app/core/components/Header.js index 54d0c402c..ca22fe12f 100644 --- a/ui/src/app/core/components/Header.js +++ b/ui/src/app/core/components/Header.js @@ -6,7 +6,7 @@ import Navbar from 'react-bootstrap/Navbar'; import Dropdown from 'react-bootstrap/Dropdown'; import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; -import { faTh, faSignOutAlt, faPlusCircle, faCube, faCubes } from '@fortawesome/free-solid-svg-icons'; +import { faTh, faSignOutAlt, faPlusCircle, faCube, faCubes, faUsersCog } from '@fortawesome/free-solid-svg-icons'; import Translate from '../../i18n/components/translate'; import { useTranslator } from '../../i18n/hooks'; @@ -41,6 +41,10 @@ export function Header () { + + + + } diff --git a/ui/src/app/form/Schema.js b/ui/src/app/form/Schema.js new file mode 100644 index 000000000..92aefe325 --- /dev/null +++ b/ui/src/app/form/Schema.js @@ -0,0 +1,24 @@ +import React from 'react'; +import useFetch from 'use-http'; + +export function Schema({ path, children }) { + + const [schema, setSchema] = React.useState({}); + + + const { get, response } = useFetch(path, { + cachePolicy: 'no-cache' + }); + + async function loadSchema() { + const list = await get(``); + if (response.ok) { + setSchema(list); + } + } + + /*eslint-disable react-hooks/exhaustive-deps*/ + React.useEffect(() => { loadSchema() }, []); + + return (<>{children(schema)}); +} \ No newline at end of file diff --git a/ui/src/app/metadata/domain/filter/component/MetadataFilters.js b/ui/src/app/metadata/domain/filter/component/MetadataFilters.js index 13a22b53c..4e5ff8a98 100644 --- a/ui/src/app/metadata/domain/filter/component/MetadataFilters.js +++ b/ui/src/app/metadata/domain/filter/component/MetadataFilters.js @@ -1,6 +1,6 @@ import React from 'react'; import { useMetadataFilters } from '../../../hooks/api'; -import { DeleteConfirmation } from '../../../component/DeleteConfirmation'; +import { DeleteConfirmation } from '../../../../core/components/DeleteConfirmation'; export const MetadataFiltersContext = React.createContext(); 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) { diff --git a/ui/src/app/metadata/view/MetadataAttributeList.js b/ui/src/app/metadata/view/MetadataAttributeList.js index b72c3c0e7..bc912118a 100644 --- a/ui/src/app/metadata/view/MetadataAttributeList.js +++ b/ui/src/app/metadata/view/MetadataAttributeList.js @@ -8,7 +8,7 @@ import { Link } from 'react-router-dom'; import { Translate } from '../../i18n/components/translate'; import { useTranslator } from '../../i18n/hooks'; -import { DeleteConfirmation } from '../component/DeleteConfirmation'; +import { DeleteConfirmation } from '../../core/components/DeleteConfirmation'; export function MetadataAttributeList ({entities, onDelete}) { From 6fb266318fb192a1cdbbaedb19bf0359e5753b1e Mon Sep 17 00:00:00 2001 From: Ryan Mathis Date: Tue, 22 Jun 2021 15:29:16 -0700 Subject: [PATCH 02/61] Added groups management code behind --- ui/public/group.json | 3 + ui/src/app/admin/component/GroupForm.js | 20 ++-- ui/src/app/admin/container/EditGroup.js | 78 ++++++++++++- ui/src/app/admin/container/GroupsList.js | 2 +- ui/src/app/admin/container/NewGroup.js | 20 +++- ui/src/app/admin/hoc/GroupProvider.js | 24 ++++ ui/src/app/admin/hooks.js | 6 +- ui/src/app/form/FormManager.js | 107 ++++++++++++++++++ .../app/metadata/editor/MetadataEditorNav.js | 3 - 9 files changed, 239 insertions(+), 24 deletions(-) create mode 100644 ui/public/group.json create mode 100644 ui/src/app/admin/hoc/GroupProvider.js create mode 100644 ui/src/app/form/FormManager.js diff --git a/ui/public/group.json b/ui/public/group.json new file mode 100644 index 000000000..07a21daa2 --- /dev/null +++ b/ui/public/group.json @@ -0,0 +1,3 @@ +{ + "name": "foo" +} \ No newline at end of file diff --git a/ui/src/app/admin/component/GroupForm.js b/ui/src/app/admin/component/GroupForm.js index 85a08d587..2f738c321 100644 --- a/ui/src/app/admin/component/GroupForm.js +++ b/ui/src/app/admin/component/GroupForm.js @@ -8,20 +8,18 @@ import Translate from '../../i18n/components/translate'; import { useGroupUiSchema } from '../hooks'; import { fields, widgets } from '../../form/component'; import { templates } from '../../form/component'; +import { FormContext, setFormDataAction } from '../../form/FormManager'; function ErrorListTemplate() { return (<>); } -export function GroupForm ({schema}) { +export function GroupForm ({group = {}, errors = [], loading = false, schema, onSave, onCancel}) { - const [errors, setErrors] = React.useState([]); - const [loading, setLoading] = React.useState(false); - const [metadata, setMetadata] = React.useState({}); - - const save = () => { }; - const cancel = () => { }; - const onChange = () => { }; + const { dispatch } = React.useContext(FormContext); + const onChange = ({formData}) => { + dispatch(setFormDataAction(formData)); + }; const uiSchema = useGroupUiSchema(); @@ -31,7 +29,7 @@ export function GroupForm ({schema}) { @@ -47,7 +45,7 @@ export function GroupForm ({schema}) {
-
onChange(form)} schema={schema} diff --git a/ui/src/app/admin/container/EditGroup.js b/ui/src/app/admin/container/EditGroup.js index 355a1d6e2..eedc86e6d 100644 --- a/ui/src/app/admin/container/EditGroup.js +++ b/ui/src/app/admin/container/EditGroup.js @@ -1,5 +1,79 @@ import React from 'react'; -export function EditGroup () { - return (<>Edit Group); +import { Prompt, useHistory } from 'react-router'; +import { useParams } from 'react-router-dom'; +import Translate from '../../i18n/components/translate'; +import { useGroups } from '../hooks'; +import { Schema } from '../../form/Schema'; +import { FormManager } from '../../form/FormManager'; + +import { GroupForm } from '../component/GroupForm'; +import { GroupProvider } from '../hoc/GroupProvider'; + +export function EditGroup() { + + const { id } = useParams(); + + const history = useHistory(); + + const { post, response, loading } = useGroups({}); + + const [blocking, setBlocking] = React.useState(false); + + async function save(metadata) { + await post(``, metadata); + if (response.ok) { + gotoDetail({ refresh: true }); + } + }; + + const cancel = () => { + gotoDetail(); + }; + + const gotoDetail = (state = null) => { + setBlocking(false); + history.push(`/groups`, state); + }; + + return ( +
+ + `message.unsaved-editor` + } + /> +
+
+
+
+ Add a new group +
+
+
+
+ + {(group) => + + {(schema) => + <>{group && + + {(data, errors) => + save(data)} + onCancel={() => cancel()} />} + + }} + + } + +
+
+
+ ); } \ No newline at end of file diff --git a/ui/src/app/admin/container/GroupsList.js b/ui/src/app/admin/container/GroupsList.js index 023b86ff5..b94a3019f 100644 --- a/ui/src/app/admin/container/GroupsList.js +++ b/ui/src/app/admin/container/GroupsList.js @@ -51,7 +51,7 @@ export function GroupsList({ groups, onDelete }) { {group.name} - + Edit diff --git a/ui/src/app/admin/container/NewGroup.js b/ui/src/app/admin/container/NewGroup.js index ab5e7d3ee..1fabd0508 100644 --- a/ui/src/app/admin/container/NewGroup.js +++ b/ui/src/app/admin/container/NewGroup.js @@ -4,7 +4,7 @@ import { Prompt, useHistory } from 'react-router'; import Translate from '../../i18n/components/translate'; import { useGroups } from '../hooks'; import { Schema } from '../../form/Schema'; - +import { FormManager } from '../../form/FormManager'; import { GroupForm } from '../component/GroupForm'; export function NewGroup() { @@ -14,8 +14,8 @@ export function NewGroup() { const [blocking, setBlocking] = React.useState(false); - async function save(metadata) { - await post(``, metadata); + async function save(group) { + await post(``, group); if (response.ok) { gotoDetail({ refresh: true }); } @@ -30,8 +30,6 @@ export function NewGroup() { history.push(`/groups`, state); }; - const [group, setGroup] = React.useState({}); - return (
- {(schema) => } + {(schema) => + + {(data, errors) => + save(data)} + onCancel={() => cancel()} />} + }
diff --git a/ui/src/app/admin/hoc/GroupProvider.js b/ui/src/app/admin/hoc/GroupProvider.js new file mode 100644 index 000000000..83674e51e --- /dev/null +++ b/ui/src/app/admin/hoc/GroupProvider.js @@ -0,0 +1,24 @@ +import React from 'react'; +import { useGroup } from '../hooks'; + +export function GroupProvider({ id, children }) { + + const [group, setGroup] = React.useState(); + + + const { get, response } = useGroup({ + cachePolicy: 'no-cache' + }); + + async function loadGroup() { + const group = await get(``); + if (response.ok) { + setGroup(group); + } + } + + /*eslint-disable react-hooks/exhaustive-deps*/ + React.useEffect(() => { loadGroup() }, []); + + return (<>{children(group)}); +} \ No newline at end of file diff --git a/ui/src/app/admin/hooks.js b/ui/src/app/admin/hooks.js index d071fb5e4..96a391944 100644 --- a/ui/src/app/admin/hooks.js +++ b/ui/src/app/admin/hooks.js @@ -6,9 +6,13 @@ export function useGroups () { } export function useGroup() { - return useFetch(`${API_BASE_PATH}/group`); + return useFetch(`/group.json`); } +/*export function useGroup() { + return useFetch(`${API_BASE_PATH}/group`); +}*/ + export function useGroupUiSchema () { return { diff --git a/ui/src/app/form/FormManager.js b/ui/src/app/form/FormManager.js new file mode 100644 index 000000000..17691970c --- /dev/null +++ b/ui/src/app/form/FormManager.js @@ -0,0 +1,107 @@ +import React from 'react'; + +const initialState = { + data: {}, + errors: [] +}; + +const FormContext = React.createContext(); + +const { Provider, Consumer } = FormContext; + +export const FormActions = { + SET_FORM_ERROR: 'set form error', + SET_FORM_DATA: 'set form data' +}; + +export const setFormDataAction = (payload) => { + return { + type: FormActions.SET_FORM_DATA, + payload + } +} + +export const setFormErrorAction = (errors) => { + return { + type: FormActions.SET_FORM_ERROR, + payload: errors + } +} + +function reducer(state, action) { + switch (action.type) { + + case FormActions.SET_FORM_ERROR: + return { + ...state, + errors: action.payload + }; + case FormActions.SET_FORM_DATA: + return { + ...state, + data: action.payload + }; + default: + return state; + } +} + +/*eslint-disable react-hooks/exhaustive-deps*/ +function FormManager({ children, initial = {} }) { + + const data = { + ...initial + }; + + const [state, dispatch] = React.useReducer(reducer, { + ...initialState, + data + }); + + + const contextValue = React.useMemo(() => ({ state, dispatch }), [state, dispatch]); + + return ( + + {children(state.data, state.errors)} + + ); +} + +function useFormErrors() { + const { state } = React.useContext(FormContext); + const { errors } = state; + + return errors; +} + +function useFormContext() { + return React.useContext(FormContext); +} + +function useFormDispatcher() { + const { dispatch } = useFormContext(); + return dispatch; +} + +function useFormState() { + const { state } = useFormContext(); + return state; +} + +function useFormData() { + const { data } = useFormContext(); + return data; +} + +export { + useFormErrors, + useFormContext, + useFormDispatcher, + useFormState, + useFormData, + FormManager, + FormContext, + Provider as FormProvider, + Consumer as FormConsumer +}; \ No newline at end of file diff --git a/ui/src/app/metadata/editor/MetadataEditorNav.js b/ui/src/app/metadata/editor/MetadataEditorNav.js index bfae1bb73..61a67a510 100644 --- a/ui/src/app/metadata/editor/MetadataEditorNav.js +++ b/ui/src/app/metadata/editor/MetadataEditorNav.js @@ -5,15 +5,12 @@ import Dropdown from 'react-bootstrap/Dropdown'; import Button from 'react-bootstrap/Button'; import Translate from '../../i18n/components/translate'; -// import { usePagesWithErrors } from '../hoc/MetadataFormContext'; export function MetadataEditorNav ({ definition, current, children, format = 'tabs', onNavigate }) { const [routes, setRoutes] = React.useState([]); const [active, setActive] = React.useState(null); - // const errors = usePagesWithErrors(definition); - React.useEffect(() => { setRoutes(definition ? definition.steps.map(step => ({ path: step.id, label: step.label })) : []) }, [definition]); From c1113339db07b526e55da24a26017ebc59fae97a Mon Sep 17 00:00:00 2001 From: chasegawa Date: Wed, 23 Jun 2021 16:36:15 -0700 Subject: [PATCH 03/61] 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 04/61] 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 05/61] 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 06/61] 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 07/61] 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()) From 2c6ff280837d948745beac0a2c0578087b4c166d Mon Sep 17 00:00:00 2001 From: Ryan Mathis Date: Mon, 28 Jun 2021 14:10:39 -0700 Subject: [PATCH 08/61] Integrated groups with backend --- .../main/resources/i18n/messages.properties | 12 +++++++++++- ui/public/assets/schema/groups/group.json | 7 +++++++ ui/src/app/admin/component/GroupForm.js | 2 +- ui/src/app/admin/container/EditGroup.js | 6 +++--- ui/src/app/admin/container/GroupsList.js | 7 ++----- ui/src/app/admin/hoc/GroupProvider.js | 6 +----- ui/src/app/admin/hoc/GroupsProvider.js | 8 ++------ ui/src/app/admin/hooks.js | 18 ++++++++++-------- 8 files changed, 37 insertions(+), 29 deletions(-) diff --git a/backend/src/main/resources/i18n/messages.properties b/backend/src/main/resources/i18n/messages.properties index 50049f5cb..bf32a0ae7 100644 --- a/backend/src/main/resources/i18n/messages.properties +++ b/backend/src/main/resources/i18n/messages.properties @@ -61,8 +61,10 @@ action.user-role=User Role action.toggle-view=Toggle view action.advanced=Advanced action.add-new-attribute=Add new attribute +action.add-new-group=Add new group action.add-attribute=Add Attribute action.custom-entity-attributes=Custom Entity Attributes +action.groups=Groups value.enabled=Enabled value.disabled=Disabled @@ -145,6 +147,9 @@ label.entity-attributes=Entity Attributes label.custom-entity-attributes=Custom Entity Attributes label.help-text=Help text label.default-value=Default Value +label.groups-management=Groups Management +label.new-group=New Group +label.new-attribute=New Custom Entity Attribute label.metadata-source=Metadata Source label.metadata-sources=Metadata Sources @@ -168,6 +173,8 @@ label.new-endpoint=New Endpoint label.select-binding=Select Binding Type label.mark-as-default=Mark as Default label.attribute-name=Attribute Name +label.group-name=Group Name +label.group-description=Group Description label.yes=Yes label.check-all-attributes=Check All Attributes label.clear-all-attributes=Clear All Attributes @@ -644,4 +651,7 @@ tooltip.match=A regular expression against which the entityID is evaluated. tooltip.remove-existing-formats=Whether to remove any existing formats from a role if any are added by the filter (unmodified roles will be untouched regardless of this setting) tooltip.nameid-formats-format=Format tooltip.nameid-formats-value=Value -tooltip.nameid-formats-type=Type \ No newline at end of file +tooltip.nameid-formats-type=Type + +tooltip.group-name=Group Name +tooltip.group-description=Group Description \ No newline at end of file diff --git a/ui/public/assets/schema/groups/group.json b/ui/public/assets/schema/groups/group.json index 088d861f6..02bcb2dea 100644 --- a/ui/public/assets/schema/groups/group.json +++ b/ui/public/assets/schema/groups/group.json @@ -10,6 +10,13 @@ "type": "string", "minLength": 1, "maxLength": 255 + }, + "description": { + "title": "label.group-description", + "description": "tooltip.group-description", + "type": "string", + "minLength": 1, + "maxLength": 255 } } } \ No newline at end of file diff --git a/ui/src/app/admin/component/GroupForm.js b/ui/src/app/admin/component/GroupForm.js index 2f738c321..37e3b73dc 100644 --- a/ui/src/app/admin/component/GroupForm.js +++ b/ui/src/app/admin/component/GroupForm.js @@ -44,7 +44,7 @@ export function GroupForm ({group = {}, errors = [], loading = false, schema, on
-
+
onChange(form)} diff --git a/ui/src/app/admin/container/EditGroup.js b/ui/src/app/admin/container/EditGroup.js index eedc86e6d..40e2a61e1 100644 --- a/ui/src/app/admin/container/EditGroup.js +++ b/ui/src/app/admin/container/EditGroup.js @@ -16,12 +16,12 @@ export function EditGroup() { const history = useHistory(); - const { post, response, loading } = useGroups({}); + const { put, response, loading } = useGroups(); const [blocking, setBlocking] = React.useState(false); async function save(metadata) { - await post(``, metadata); + await put(``, metadata); if (response.ok) { gotoDetail({ refresh: true }); } @@ -48,7 +48,7 @@ export function EditGroup() {
- Add a new group + Edit group
diff --git a/ui/src/app/admin/container/GroupsList.js b/ui/src/app/admin/container/GroupsList.js index b94a3019f..f0a1146e3 100644 --- a/ui/src/app/admin/container/GroupsList.js +++ b/ui/src/app/admin/container/GroupsList.js @@ -6,14 +6,11 @@ import Button from 'react-bootstrap/Button'; import { Link } from 'react-router-dom'; import { Translate } from '../../i18n/components/translate'; -import { useTranslator } from '../../i18n/hooks'; import { DeleteConfirmation } from '../../core/components/DeleteConfirmation'; export function GroupsList({ groups, onDelete }) { - const translator = useTranslator(); - const remove = (id) => { onDelete(id); } @@ -51,13 +48,13 @@ export function GroupsList({ groups, onDelete }) { {group.name} - + Edit - - } - - - )} + + {(groups, onRemove, loadingGroups) => + + {users.map((user, idx) => + + {user.username} + {user.firstName} {user.lastName} + {user.emailAddress} + + + + + + + + + + {currentUser.username !== user.username && + + } + + + )} + + } +
diff --git a/ui/src/app/admin/container/GroupsList.js b/ui/src/app/admin/container/GroupsList.js index f0a1146e3..07090f412 100644 --- a/ui/src/app/admin/container/GroupsList.js +++ b/ui/src/app/admin/container/GroupsList.js @@ -38,15 +38,19 @@ export function GroupsList({ groups, onDelete }) { - Group Name + Group Name + + + Group Description Actions - {groups?.length && groups.map((group, i) => + {(groups?.length > 0 ) ? groups.map((group, i) => {group.name} + {group.description} @@ -62,7 +66,7 @@ export function GroupsList({ groups, onDelete }) { - )} + ) : ''}
diff --git a/ui/src/app/admin/container/UserManagement.js b/ui/src/app/admin/container/UserManagement.js index e4e1140ef..b8e8e6784 100644 --- a/ui/src/app/admin/container/UserManagement.js +++ b/ui/src/app/admin/container/UserManagement.js @@ -38,6 +38,19 @@ export default function UserManagement({ users, children, reload }) { } } + async function setUserGroupRequest(user, groupId) { + await patch(`/admin/users/${user.username}`, { + ...user, + groupId + }); + if (response.ok && reload) { + dispatch(createNotificationAction( + `User update successful for ${user.username}.` + )); + reload(); + } + } + async function deleteUserRequest(id) { await del(`/admin/users/${id}`); if (response.ok && reload) { @@ -66,7 +79,7 @@ export default function UserManagement({ users, children, reload }) { return (
- {children(users, roles, setUserRoleRequest, (id) => setDeleting(id))} + {children(users, roles, setUserRoleRequest, setUserGroupRequest, (id) => setDeleting(id))} setDeleting(null)}> Delete User? diff --git a/ui/src/app/admin/hoc/GroupsProvider.js b/ui/src/app/admin/hoc/GroupsProvider.js index c7243e1fa..256235b05 100644 --- a/ui/src/app/admin/hoc/GroupsProvider.js +++ b/ui/src/app/admin/hoc/GroupsProvider.js @@ -1,13 +1,13 @@ import React from 'react'; import { useGroups } from '../hooks'; -export function GroupsProvider({ children }) { +export function GroupsProvider({ children, cache = 'no-cache' }) { const [groups, setGroups] = React.useState([]); - const { get, del, response } = useGroups({ - cachePolicy: 'no-cache' + const { get, del, response, loading } = useGroups({ + cachePolicy: cache }); async function loadGroups() { @@ -27,5 +27,5 @@ export function GroupsProvider({ children }) { /*eslint-disable react-hooks/exhaustive-deps*/ React.useEffect(() => { loadGroups() }, []); - return (<>{children(groups, removeGroup)}); + return (<>{children(groups, removeGroup, loading)}); } \ No newline at end of file diff --git a/ui/src/app/admin/hooks.js b/ui/src/app/admin/hooks.js index 2d2b70610..28aef201a 100644 --- a/ui/src/app/admin/hooks.js +++ b/ui/src/app/admin/hooks.js @@ -1,10 +1,8 @@ import useFetch from 'use-http'; import API_BASE_PATH from '../App.constant'; -export function useGroups () { - return useFetch(`${API_BASE_PATH}/admin/groups`, { - cachePolicy: 'no-cache' - }); +export function useGroups (opts = { cachePolicy: 'no-cache' }) { + return useFetch(`${API_BASE_PATH}/admin/groups`, opts); } export function useGroup(id) { diff --git a/ui/src/app/dashboard/view/AdminTab.js b/ui/src/app/dashboard/view/AdminTab.js index a458c810a..268ea61fc 100644 --- a/ui/src/app/dashboard/view/AdminTab.js +++ b/ui/src/app/dashboard/view/AdminTab.js @@ -39,8 +39,8 @@ export function AdminTab () {
- {(u, roles, onChangeUserRole, onDeleteUser) => - } + {(u, roles, onChangeUserRole, onChangeUserGroup, onDeleteUser) => + }
From b592f389a8139f7a0f564736cefd5e8046b479af Mon Sep 17 00:00:00 2001 From: chasegawa Date: Fri, 2 Jul 2021 10:30:56 -0700 Subject: [PATCH 14/61] SHIBUI-1740 fixing bug with users/roles due to hashcode --- .../ui/security/controller/UsersController.java | 10 +++++++++- .../shibboleth/admin/ui/security/model/User.java | 15 +++++++++++++++ .../ui/security/repository/UserRepository.java | 1 - 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/controller/UsersController.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/controller/UsersController.java index 3b75b865c..027de990a 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/controller/UsersController.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/controller/UsersController.java @@ -8,6 +8,7 @@ import edu.internet2.tier.shibboleth.admin.ui.security.repository.UserRepository; import edu.internet2.tier.shibboleth.admin.ui.security.service.UserService; import groovy.util.logging.Slf4j; +import jline.internal.Log; import org.apache.commons.lang.StringUtils; import org.springframework.beans.factory.annotation.Autowired; @@ -67,7 +68,14 @@ private User findUserOrThrowHttp404(String username) { @Transactional(readOnly = true) @GetMapping public List getAll() { - return userRepository.findAll(); + try { + List results = userRepository.findAll(); + return results; + } + catch (Exception e) { + Log.error("Unable to fetch users because: {}", e.getMessage()); + throw e; + } } @Transactional(readOnly = true) 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 8acb53a45..e967c104d 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 @@ -58,6 +58,7 @@ public class User extends AbstractAuditable { private String password; @Transient + @EqualsAndHashCode.Exclude private String role; @JsonIgnore @@ -69,6 +70,13 @@ public class User extends AbstractAuditable { @Column(nullable = false, unique = true) private String username; + public String getGroupId() { + if (groupId == null && group != null) { + groupId = group.getResourceId(); + } + return groupId; + } + public String getRole() { if (StringUtils.isBlank(this.role)) { Set roles = this.getRoles(); @@ -79,4 +87,11 @@ public String getRole() { } return this.role; } + + public void setGroup(Group assignedGroup) { + this.group = assignedGroup; + if (group != null) { + groupId = group.getResourceId(); + } + } } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/repository/UserRepository.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/repository/UserRepository.java index 380e8501a..0f1f7f201 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/repository/UserRepository.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/repository/UserRepository.java @@ -12,7 +12,6 @@ * @author Dmitriy Kopylenko */ public interface UserRepository extends JpaRepository { - Optional findByUsername(String username); Set findByRoles_Name(String roleName); } From c8b5eaa4f05fe9b1936ffadb757d228f6b1d6fa8 Mon Sep 17 00:00:00 2001 From: Ryan Mathis Date: Fri, 2 Jul 2021 11:28:49 -0700 Subject: [PATCH 15/61] Fixed dropsdowns for groups --- .../main/resources/i18n/messages.properties | 1 + ui/src/app/admin/component/UserMaintenance.js | 4 +- ui/src/app/core/user/UserContext.js | 13 ++++- ui/src/app/dashboard/view/SourcesTab.js | 18 ++++++- .../domain/source/component/SourceList.js | 48 +++++++++++++++---- 5 files changed, 70 insertions(+), 14 deletions(-) diff --git a/backend/src/main/resources/i18n/messages.properties b/backend/src/main/resources/i18n/messages.properties index 293b00d5c..9f3e5b20f 100644 --- a/backend/src/main/resources/i18n/messages.properties +++ b/backend/src/main/resources/i18n/messages.properties @@ -480,6 +480,7 @@ label.current=Current label.restore=Restore label.compare-selected=Compare Selected label.restore-version=Restore Version ({ date }) +label.group=Group label.saved=Saved label.by=By diff --git a/ui/src/app/admin/component/UserMaintenance.js b/ui/src/app/admin/component/UserMaintenance.js index bb9faac45..6bb40810d 100644 --- a/ui/src/app/admin/component/UserMaintenance.js +++ b/ui/src/app/admin/component/UserMaintenance.js @@ -51,13 +51,13 @@ export default function UserMaintenance({ users, roles, onDeleteUser, onChangeUs - + onChangeGroup(source, event.target.value)} + value={source.groupId ? source.groupId : ''} + disabled={loadingGroups} + disablevalidation="true"> + + {groups.map((g, ridx) => ( + + ))} + + + } + + + } + {onDeleteSource && + From c356817ae387e763688fb06bbe7ec20d8de6a61c Mon Sep 17 00:00:00 2001 From: chasegawa Date: Sat, 3 Jul 2021 00:28:42 -0700 Subject: [PATCH 16/61] SHIBUI-1848 Refactoring EntityDescriptorController - moving as much logic for permission checking and error handling out of the controller to the service layer and ExceptionHandler --- .../EntityDescriptorController.java | 114 +++++------------- ...yDescriptorControllerExceptionHandler.java | 44 +++++++ .../ui/exception/EntityIdExistsException.java | 8 ++ .../ui/exception/EntityNotFoundException.java | 7 ++ .../ui/exception/ForbiddenException.java | 11 ++ .../ui/security/service/UserService.java | 19 ++- .../ui/service/EntityDescriptorService.java | 14 ++- .../JPAEntityDescriptorServiceImpl.java | 60 ++++++++- .../EntityDescriptorControllerTests.groovy | 75 ++++++------ 9 files changed, 224 insertions(+), 128 deletions(-) create mode 100644 backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorControllerExceptionHandler.java create mode 100644 backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/exception/EntityIdExistsException.java create mode 100644 backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/exception/EntityNotFoundException.java create mode 100644 backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/exception/ForbiddenException.java diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorController.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorController.java index f952b2a4d..a7e74947f 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorController.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorController.java @@ -3,12 +3,16 @@ import edu.internet2.tier.shibboleth.admin.ui.domain.EntityDescriptor; import edu.internet2.tier.shibboleth.admin.ui.domain.frontend.EntityDescriptorRepresentation; import edu.internet2.tier.shibboleth.admin.ui.domain.versioning.Version; +import edu.internet2.tier.shibboleth.admin.ui.exception.EntityIdExistsException; +import edu.internet2.tier.shibboleth.admin.ui.exception.EntityNotFoundException; +import edu.internet2.tier.shibboleth.admin.ui.exception.ForbiddenException; 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.security.model.User; import edu.internet2.tier.shibboleth.admin.ui.security.service.UserService; import edu.internet2.tier.shibboleth.admin.ui.service.EntityDescriptorService; import edu.internet2.tier.shibboleth.admin.ui.service.EntityDescriptorVersionService; +import edu.internet2.tier.shibboleth.admin.ui.service.EntityService; import lombok.extern.slf4j.Slf4j; import org.opensaml.core.xml.io.MarshallingException; @@ -20,6 +24,7 @@ import org.springframework.security.access.annotation.Secured; import org.springframework.transaction.annotation.Transactional; import org.springframework.web.bind.annotation.DeleteMapping; +import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.PostMapping; @@ -32,7 +37,9 @@ import org.springframework.web.servlet.support.ServletUriComponentsBuilder; import javax.annotation.PostConstruct; + import java.net.URI; +import java.util.ConcurrentModificationException; import java.util.List; import java.util.stream.Collectors; @@ -40,14 +47,14 @@ @RequestMapping("/api") @Slf4j public class EntityDescriptorController { - private static URI getResourceUriFor(EntityDescriptor ed) { + static URI getResourceUriFor(String resourceId) { return ServletUriComponentsBuilder .fromCurrentServletMapping().path("/api/EntityDescriptor") - .pathSegment(ed.getResourceId()) + .pathSegment(resourceId) .build() .toUri(); } - + @Autowired private EntityDescriptorRepository entityDescriptorRepository; @@ -73,58 +80,28 @@ public EntityDescriptorController(UserService userService, EntityDescriptorVersi @PostMapping("/EntityDescriptor") @Transactional - public ResponseEntity create(@RequestBody EntityDescriptorRepresentation edRepresentation) { - final String entityId = edRepresentation.getEntityId(); - - ResponseEntity entityDescriptorEnablingDeniedResponse = entityDescriptorEnablePermissionsCheck(edRepresentation.isServiceEnabled()); - if (entityDescriptorEnablingDeniedResponse != null) { - return entityDescriptorEnablingDeniedResponse; - } - - ResponseEntity existingEntityDescriptorConflictResponse = existingEntityDescriptorCheck(entityId); - if (existingEntityDescriptorConflictResponse != null) { - return existingEntityDescriptorConflictResponse; - } - - EntityDescriptor ed = (EntityDescriptor) entityDescriptorService.createDescriptorFromRepresentation(edRepresentation); - - EntityDescriptor persistedEd = entityDescriptorRepository.save(ed); - edRepresentation.setId(persistedEd.getResourceId()); - edRepresentation.setCreatedDate(persistedEd.getCreatedDate()); - return ResponseEntity.created(getResourceUriFor(persistedEd)).body(entityDescriptorService.createRepresentationFromDescriptor(persistedEd)); + public ResponseEntity create(@RequestBody EntityDescriptorRepresentation edRepresentation) throws ForbiddenException, EntityIdExistsException { + EntityDescriptorRepresentation persistedEd = entityDescriptorService.createNew(edRepresentation); + return ResponseEntity.created(getResourceUriFor(persistedEd.getId())).body(persistedEd); } @Secured("ROLE_ADMIN") @DeleteMapping(value = "/EntityDescriptor/{resourceId}") @Transactional - public ResponseEntity deleteOne(@PathVariable String resourceId) { - EntityDescriptor ed = entityDescriptorRepository.findByResourceId(resourceId); - if (ed == null) { - return ResponseEntity.notFound().build(); - } else if (ed.isServiceEnabled()) { - return ResponseEntity.status(HttpStatus.FORBIDDEN).body(new ErrorResponse(HttpStatus.FORBIDDEN, "Deleting an enabled Metadata Source is not allowed. Disable the source and try again.")); - } else { - entityDescriptorRepository.delete(ed); - return ResponseEntity.noContent().build(); - } - } - - private ResponseEntity entityDescriptorEnablePermissionsCheck(boolean serviceEnabled) { - User user = userService.getCurrentUser(); - if (user != null) { - if (serviceEnabled && !user.getRole().equals("ROLE_ADMIN")) { - return ResponseEntity.status(HttpStatus.FORBIDDEN) - .body(new ErrorResponse(HttpStatus.FORBIDDEN, "You do not have the permissions necessary to enable this service.")); - } + public ResponseEntity deleteOne(@PathVariable String resourceId) throws ForbiddenException, EntityNotFoundException { + EntityDescriptor ed = entityDescriptorService.getEntityDescriptorByResourceId(resourceId); + if (ed.isServiceEnabled()) { + throw new ForbiddenException("Deleting an enabled Metadata Source is not allowed. Disable the source and try again."); } - return null; + entityDescriptorRepository.delete(ed); + return ResponseEntity.noContent().build(); } private ResponseEntity existingEntityDescriptorCheck(String entityId) { final EntityDescriptor ed = entityDescriptorRepository.findByEntityID(entityId); if (ed != null) { HttpHeaders headers = new HttpHeaders(); - headers.setLocation(getResourceUriFor(ed)); + headers.setLocation(getResourceUriFor(ed.getResourceId())); return ResponseEntity .status(HttpStatus.CONFLICT) .headers(headers) @@ -137,12 +114,10 @@ private ResponseEntity existingEntityDescriptorCheck(String entityId) { @GetMapping("/EntityDescriptors") @Transactional(readOnly = true) public ResponseEntity getAll() { - User currentUser = userService.getCurrentUser(); - if (currentUser != null) { + try { return ResponseEntity.ok(entityDescriptorService.getAllRepresentationsBasedOnUserAccess()); - } else { - return ResponseEntity.status(HttpStatus.FORBIDDEN).body(new ErrorResponse(HttpStatus.FORBIDDEN, - "You are not authorized to perform the requested operation.")); + } catch (ForbiddenException e) { + return ResponseEntity.status(HttpStatus.FORBIDDEN).body(new ErrorResponse(HttpStatus.FORBIDDEN, e.getMessage())); } } @@ -225,6 +200,11 @@ public ResponseEntity getSpecificVersion(@PathVariable String resourceId, @Pa return ResponseEntity.status(HttpStatus.FORBIDDEN).build(); } + @ExceptionHandler({ ForbiddenException.class }) + public void handleException() { + // + } + private ResponseEntity handleUploadingEntityDescriptorXml(byte[] rawXmlBytes, String spName) throws Exception { final EntityDescriptor ed = EntityDescriptor.class.cast(openSamlObjects.unmarshalFromXml(rawXmlBytes)); @@ -235,7 +215,7 @@ private ResponseEntity handleUploadingEntityDescriptorXml(byte[] rawXmlBytes, ed.setServiceProviderName(spName); final EntityDescriptor persistedEd = entityDescriptorRepository.save(ed); - return ResponseEntity.created(getResourceUriFor(persistedEd)) + return ResponseEntity.created(getResourceUriFor(persistedEd.getResourceId())) .body(entityDescriptorService.createRepresentationFromDescriptor(persistedEd)); } @@ -246,38 +226,10 @@ public void initRestTemplate() { @PutMapping("/EntityDescriptor/{resourceId}") @Transactional - public ResponseEntity update(@RequestBody EntityDescriptorRepresentation edRepresentation, @PathVariable String resourceId) { - User currentUser = userService.getCurrentUser(); - if (currentUser != null) { - EntityDescriptor existingEd = entityDescriptorRepository.findByResourceId(resourceId); - - if (existingEd == null) { - return ResponseEntity.notFound().build(); - } else { - if (userService.isAuthorizedFor(existingEd.getCreatedBy(), - existingEd.getGroup() == null ? null : existingEd.getGroup().getResourceId())) { - if (!existingEd.isServiceEnabled()) { - ResponseEntity entityDescriptorEnablingDeniedResponse = entityDescriptorEnablePermissionsCheck( - edRepresentation.isServiceEnabled()); - if (entityDescriptorEnablingDeniedResponse != null) { - return entityDescriptorEnablingDeniedResponse; - } - } - - // Verify we're the only one attempting to update the EntityDescriptor - if (edRepresentation.getVersion() != existingEd.hashCode()) { - return new ResponseEntity(HttpStatus.CONFLICT); - } - - entityDescriptorService.updateDescriptorFromRepresentation(existingEd, edRepresentation); - existingEd = entityDescriptorRepository.save(existingEd); - - return ResponseEntity.ok().body(entityDescriptorService.createRepresentationFromDescriptor(existingEd)); - } - } - } - return ResponseEntity.status(HttpStatus.FORBIDDEN).body(new ErrorResponse(HttpStatus.FORBIDDEN, - "You are not authorized to perform the requested operation.")); + public ResponseEntity update(@RequestBody EntityDescriptorRepresentation edRepresentation, @PathVariable String resourceId) throws ForbiddenException, ConcurrentModificationException, EntityNotFoundException { + edRepresentation.setId(resourceId); // This should be the same already, but just to be safe... + EntityDescriptorRepresentation result = entityDescriptorService.update(edRepresentation); + return ResponseEntity.ok().body(result); } @PostMapping(value = "/EntityDescriptor", consumes = "application/xml") diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorControllerExceptionHandler.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorControllerExceptionHandler.java new file mode 100644 index 000000000..f89c16f8c --- /dev/null +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorControllerExceptionHandler.java @@ -0,0 +1,44 @@ +package edu.internet2.tier.shibboleth.admin.ui.controller; + +import java.util.ConcurrentModificationException; + +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; +import org.springframework.web.bind.annotation.ControllerAdvice; +import org.springframework.web.bind.annotation.ExceptionHandler; +import org.springframework.web.context.request.WebRequest; +import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler; + +import edu.internet2.tier.shibboleth.admin.ui.exception.EntityIdExistsException; +import edu.internet2.tier.shibboleth.admin.ui.exception.EntityNotFoundException; +import edu.internet2.tier.shibboleth.admin.ui.exception.ForbiddenException; + +@ControllerAdvice(assignableTypes = {EntityDescriptorController.class}) +public class EntityDescriptorControllerExceptionHandler extends ResponseEntityExceptionHandler { + + @ExceptionHandler({ ForbiddenException.class }) + public ResponseEntity handleForbiddenAccess(ForbiddenException e, WebRequest request) { + return ResponseEntity.status(HttpStatus.FORBIDDEN).body(new ErrorResponse(HttpStatus.FORBIDDEN, e.getMessage())); + } + + @ExceptionHandler({ EntityIdExistsException.class }) + public ResponseEntity handleEntityExistsException(EntityIdExistsException e, WebRequest request) { + HttpHeaders headers = new HttpHeaders(); + headers.setLocation(EntityDescriptorController.getResourceUriFor(e.getMessage())); + return ResponseEntity.status(HttpStatus.CONFLICT).headers(headers).body(new ErrorResponse( + String.valueOf(HttpStatus.CONFLICT.value()), + String.format("The entity descriptor with entity id [%s] already exists.", e.getMessage()))); + + } + + @ExceptionHandler({ EntityNotFoundException.class }) + public ResponseEntity handleEntityNotFoundException(EntityNotFoundException e, WebRequest request) { + return ResponseEntity.status(HttpStatus.NOT_FOUND).body(new ErrorResponse(HttpStatus.NOT_FOUND, e.getMessage())); + } + + @ExceptionHandler({ ConcurrentModificationException.class }) + public ResponseEntity handleConcurrentModificationException(ConcurrentModificationException e, WebRequest request) { + return ResponseEntity.status(HttpStatus.CONFLICT).body(new ErrorResponse(HttpStatus.CONFLICT, e.getMessage())); + } +} diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/exception/EntityIdExistsException.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/exception/EntityIdExistsException.java new file mode 100644 index 000000000..990eab2c3 --- /dev/null +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/exception/EntityIdExistsException.java @@ -0,0 +1,8 @@ +package edu.internet2.tier.shibboleth.admin.ui.exception; + +public class EntityIdExistsException extends Exception { + public EntityIdExistsException(String entityId) { + super(entityId); + } + +} diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/exception/EntityNotFoundException.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/exception/EntityNotFoundException.java new file mode 100644 index 000000000..6769aaa5f --- /dev/null +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/exception/EntityNotFoundException.java @@ -0,0 +1,7 @@ +package edu.internet2.tier.shibboleth.admin.ui.exception; + +public class EntityNotFoundException extends Exception { + public EntityNotFoundException(String message) { + super(message); + } +} diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/exception/ForbiddenException.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/exception/ForbiddenException.java new file mode 100644 index 000000000..66f2e61c5 --- /dev/null +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/exception/ForbiddenException.java @@ -0,0 +1,11 @@ +package edu.internet2.tier.shibboleth.admin.ui.exception; + +public class ForbiddenException extends Exception { + public ForbiddenException() { + super("You are not authorized to perform the requested operation."); + } + + public ForbiddenException(String message) { + super(message); + } +} diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java index 220bb090c..61684c592 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java @@ -1,5 +1,6 @@ package edu.internet2.tier.shibboleth.admin.ui.security.service; +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.RoleRepository; @@ -50,18 +51,25 @@ public UserAccess getCurrentUserAccess() { return UserAccess.OWNER; } } + + public boolean isAuthorizedFor(String objectCreatedBy, Group objectGroup) { + String groupId = objectGroup == null ? "" : objectGroup.getResourceId(); + return isAuthorizedFor(objectCreatedBy, groupId); + } + public boolean isAuthorizedFor(String objectCreatedBy, String objectGroupResourceId) { - User currentUser = getCurrentUser(); String groupId = objectGroupResourceId == null ? "" : objectGroupResourceId; - switch (getCurrentUserAccess()) { + switch (getCurrentUserAccess()) { // no user returns NONE case ADMIN: return true; case GROUP: + User currentUser = getCurrentUser(); return objectCreatedBy.equals(currentUser.getUsername()) || groupId.equals(currentUser.getGroupId()); case OWNER: - return objectCreatedBy.equals(currentUser.getUsername()); + User cu = getCurrentUser(); + return objectCreatedBy.equals(cu.getUsername()); default: return false; } @@ -89,4 +97,9 @@ public void updateUserRole(User user) { throw new RuntimeException(String.format("User with username [%s] has no role defined and therefor cannot be updated!", user.getUsername())); } } + + public boolean currentUserIsAdmin() { + User user = getCurrentUser(); + return user != null && user.getRole().equals("ROLE_ADMIN"); + } } \ No newline at end of file diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/EntityDescriptorService.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/EntityDescriptorService.java index cdcf80774..5c05d43db 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/EntityDescriptorService.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/EntityDescriptorService.java @@ -2,8 +2,13 @@ import edu.internet2.tier.shibboleth.admin.ui.domain.Attribute; import edu.internet2.tier.shibboleth.admin.ui.domain.frontend.EntityDescriptorRepresentation; +import edu.internet2.tier.shibboleth.admin.ui.exception.EntityIdExistsException; +import edu.internet2.tier.shibboleth.admin.ui.exception.EntityNotFoundException; +import edu.internet2.tier.shibboleth.admin.ui.exception.ForbiddenException; + import org.opensaml.saml.saml2.metadata.EntityDescriptor; +import java.util.ConcurrentModificationException; import java.util.List; import java.util.Map; @@ -13,7 +18,6 @@ * @since 1.0 */ public interface EntityDescriptorService { - /** * Map from front-end data representation of entity descriptor to opensaml implementation of entity descriptor model * @@ -33,7 +37,7 @@ public interface EntityDescriptorService { /** * @return a list of EntityDescriptorRepresentations that a user has the rights to access */ - List getAllRepresentationsBasedOnUserAccess(); + List getAllRepresentationsBasedOnUserAccess() throws ForbiddenException; /** * Given a list of attributes, generate an AttributeReleaseList @@ -59,4 +63,10 @@ public interface EntityDescriptorService { */ void updateDescriptorFromRepresentation(final EntityDescriptor entityDescriptor, final EntityDescriptorRepresentation representation); + EntityDescriptorRepresentation createNew(EntityDescriptorRepresentation edRepresentation) throws ForbiddenException, EntityIdExistsException; + + EntityDescriptorRepresentation update(EntityDescriptorRepresentation edRepresentation) throws ForbiddenException, EntityNotFoundException, ConcurrentModificationException; + + edu.internet2.tier.shibboleth.admin.ui.domain.EntityDescriptor getEntityDescriptorByResourceId(String resourceId) throws EntityNotFoundException; + } \ No newline at end of file diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java index 5734f55ec..feffa2fc5 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java @@ -2,6 +2,8 @@ import com.google.common.base.Strings; + +import edu.internet2.tier.shibboleth.admin.ui.controller.ErrorResponse; import edu.internet2.tier.shibboleth.admin.ui.domain.AssertionConsumerService; import edu.internet2.tier.shibboleth.admin.ui.domain.Attribute; import edu.internet2.tier.shibboleth.admin.ui.domain.AttributeBuilder; @@ -41,12 +43,14 @@ import edu.internet2.tier.shibboleth.admin.ui.domain.frontend.OrganizationRepresentation; import edu.internet2.tier.shibboleth.admin.ui.domain.frontend.SecurityInfoRepresentation; import edu.internet2.tier.shibboleth.admin.ui.domain.frontend.ServiceProviderSsoDescriptorRepresentation; +import edu.internet2.tier.shibboleth.admin.ui.exception.EntityIdExistsException; +import edu.internet2.tier.shibboleth.admin.ui.exception.EntityNotFoundException; +import edu.internet2.tier.shibboleth.admin.ui.exception.ForbiddenException; 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.security.model.Group; import edu.internet2.tier.shibboleth.admin.ui.security.model.User; import edu.internet2.tier.shibboleth.admin.ui.security.service.IGroupService; -import edu.internet2.tier.shibboleth.admin.ui.security.service.UserAccess; import edu.internet2.tier.shibboleth.admin.ui.security.service.UserService; import edu.internet2.tier.shibboleth.admin.util.MDDCConstants; import edu.internet2.tier.shibboleth.admin.util.ModelRepresentationConversions; @@ -57,13 +61,14 @@ import org.opensaml.xmlsec.signature.KeyInfo; import org.opensaml.xmlsec.signature.X509Certificate; import org.opensaml.xmlsec.signature.X509Data; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.ConcurrentModificationException; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -400,7 +405,7 @@ public EntityDescriptorRepresentation createRepresentationFromDescriptor(org.ope } @Override - public List getAllRepresentationsBasedOnUserAccess() { + public List getAllRepresentationsBasedOnUserAccess() throws ForbiddenException { switch (userService.getCurrentUserAccess()) { case ADMIN: return entityDescriptorRepository.findAllStreamByCustomQuery().map(ed -> createRepresentationFromDescriptor(ed)) @@ -412,10 +417,10 @@ public List getAllRepresentationsBasedOnUserAcce .findAllStreamByGroup_resourceIdOrCreatedBy(group == null ? null : group.getResourceId(), user.getUsername()) .map(ed -> createRepresentationFromDescriptor(ed)).collect(Collectors.toList()); case OWNER: - default: return entityDescriptorRepository.findAllStreamByCreatedBy(userService.getCurrentUser().getUsername()) .map(ed -> createRepresentationFromDescriptor(ed)).collect(Collectors.toList()); - + default: + throw new ForbiddenException(); } } @@ -738,4 +743,47 @@ public void updateDescriptorFromRepresentation(org.opensaml.saml.saml2.metadata. } buildDescriptorFromRepresentation((EntityDescriptor) entityDescriptor, representation); } + + @Override + public EntityDescriptorRepresentation createNew(EntityDescriptorRepresentation edRep) throws ForbiddenException, EntityIdExistsException { + if (edRep.isServiceEnabled() && !userService.currentUserIsAdmin()) { + throw new ForbiddenException("You do not have the permissions necessary to enable this service."); + } + + if (entityDescriptorRepository.findByEntityID(edRep.getEntityId()) != null) { + throw new EntityIdExistsException(edRep.getEntityId()); + } + + EntityDescriptor ed = (EntityDescriptor) createDescriptorFromRepresentation(edRep); + return createRepresentationFromDescriptor(entityDescriptorRepository.save(ed)); + } + + @Override + public EntityDescriptorRepresentation update(EntityDescriptorRepresentation edRep) throws ForbiddenException, EntityNotFoundException { + EntityDescriptor existingEd = entityDescriptorRepository.findByResourceId(edRep.getId()); + if (existingEd == null) { + throw new EntityNotFoundException(String.format("The entity descriptor with entity id [%s] was not found for update.", edRep.getId())); + } + if (edRep.isServiceEnabled() && !userService.currentUserIsAdmin()) { + throw new ForbiddenException("You do not have the permissions necessary to enable this service."); + } + if (!userService.isAuthorizedFor(existingEd.getCreatedBy(), existingEd.getGroup())) { + throw new ForbiddenException("You are not authorized to perform the requested operation."); + } + // Verify we're the only one attempting to update the EntityDescriptor + if (edRep.getVersion() != existingEd.hashCode()) { + throw new ConcurrentModificationException(String.format("A concurrent modification has occured on entity descriptor with entity id [%s]. Please refresh and try again", edRep.getId())); + } + updateDescriptorFromRepresentation(existingEd, edRep); + return createRepresentationFromDescriptor(entityDescriptorRepository.save(existingEd)); + } + + @Override + public EntityDescriptor getEntityDescriptorByResourceId(String resourceId) throws EntityNotFoundException { + EntityDescriptor ed = entityDescriptorRepository.findByResourceId(resourceId); + if (ed == null) { + throw new EntityNotFoundException(String.format("The entity descriptor with entity id [%s] was not found.", resourceId)); + } + return ed; + } } diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorControllerTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorControllerTests.groovy index 08f42f537..fc2c2c46c 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorControllerTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorControllerTests.groovy @@ -6,6 +6,8 @@ import edu.internet2.tier.shibboleth.admin.ui.configuration.Internationalization import edu.internet2.tier.shibboleth.admin.ui.configuration.SearchConfiguration import edu.internet2.tier.shibboleth.admin.ui.configuration.TestConfiguration import edu.internet2.tier.shibboleth.admin.ui.domain.EntityDescriptor +import edu.internet2.tier.shibboleth.admin.ui.exception.EntityIdExistsException +import edu.internet2.tier.shibboleth.admin.ui.exception.ForbiddenException 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.security.repository.RoleRepository @@ -20,8 +22,12 @@ import edu.internet2.tier.shibboleth.admin.ui.util.TestHelpers import edu.internet2.tier.shibboleth.admin.ui.util.TestObjectGenerator import groovy.json.JsonOutput import groovy.json.JsonSlurper + +import org.springframework.beans.factory.support.RootBeanDefinition import org.springframework.boot.autoconfigure.domain.EntityScan import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest +import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest +import org.springframework.context.support.StaticApplicationContext import org.springframework.data.jpa.repository.config.EnableJpaRepositories import org.springframework.security.core.Authentication import org.springframework.security.core.context.SecurityContext @@ -29,6 +35,10 @@ import org.springframework.security.core.context.SecurityContextHolder import org.springframework.test.context.ContextConfiguration import org.springframework.test.web.servlet.setup.MockMvcBuilders import org.springframework.web.client.RestTemplate +import org.springframework.web.servlet.config.annotation.EnableWebMvc +import org.springframework.web.servlet.config.annotation.WebMvcConfigurationSupport +import org.springframework.web.servlet.mvc.method.annotation.ExceptionHandlerExceptionResolver + import spock.lang.Ignore import spock.lang.Specification import spock.lang.Subject @@ -92,8 +102,8 @@ class EntityDescriptorControllerTests extends Specification { controller.restTemplate = mockRestTemplate - mockMvc = MockMvcBuilders.standaloneSetup(controller).build() - + mockMvc = MockMvcBuilders.standaloneSetup(controller).build(); + securityContext.getAuthentication() >> authentication SecurityContextHolder.setContext(securityContext) @@ -410,20 +420,18 @@ class EntityDescriptorControllerTests extends Specification { """ when: - def result = mockMvc.perform( - post('/api/EntityDescriptor') - .contentType(APPLICATION_JSON) - .content(postedJsonBody)) + def exception = mockMvc.perform(post('/api/EntityDescriptor').contentType(APPLICATION_JSON).content(postedJsonBody)).andReturn().getResolvedException() then: - 0 * entityDescriptorRepository.findByEntityID(_) - 0 * entityDescriptorRepository.save(_) - - result.andExpect(status().isForbidden()) + exception instanceof ForbiddenException == true } def 'POST /EntityDescriptor record already exists'() { given: + def username = 'admin' + def role = 'ROLE_ADMIN' + authentication.getName() >> username + userRepository.findByUsername(username) >> TestHelpers.generateOptionalUser(username, role) def expectedEntityId = 'eid1' def postedJsonBody = """ { @@ -446,15 +454,16 @@ class EntityDescriptorControllerTests extends Specification { """ when: - def result = mockMvc.perform( - post('/api/EntityDescriptor') - .contentType(APPLICATION_JSON) - .content(postedJsonBody)) + //Stub invocation of the repository returning an existing record + 1 * entityDescriptorRepository.findByEntityID(expectedEntityId) >> new EntityDescriptor(entityID: expectedEntityId) then: - //Stub invocation of the repository returning an existing record - 1 * entityDescriptorRepository.findByEntityID(expectedEntityId) >> new EntityDescriptor(entityID: expectedEntityId) - result.andExpect(status().isConflict()) + try { + def exceptionExpected = mockMvc.perform(post('/api/EntityDescriptor').contentType(APPLICATION_JSON).content(postedJsonBody)) + } + catch (Exception e) { + e instanceof EntityIdExistsException == true + } } def 'GET /EntityDescriptor/{resourceId} non-existent'() { @@ -964,13 +973,10 @@ class EntityDescriptorControllerTests extends Specification { 0 * entityDescriptorRepository.save(_) >> updatedEntityDescriptor when: - def result = mockMvc.perform( - put("/api/EntityDescriptor/$resourceId") - .contentType(APPLICATION_JSON) - .content(postedJsonBody)) + def exception = mockMvc.perform(put("/api/EntityDescriptor/$resourceId").contentType(APPLICATION_JSON).content(postedJsonBody)).andReturn().getResolvedException() then: - result.andExpect(status().isForbidden()) + exception instanceof ForbiddenException == true } def "PUT /EntityDescriptor denies the request if the PUTing user is not an ADMIN and not the createdBy user"() { @@ -990,16 +996,13 @@ class EntityDescriptorControllerTests extends Specification { 1 * entityDescriptorRepository.findByResourceId(resourceId) >> entityDescriptor when: - def result = mockMvc.perform( - put("/api/EntityDescriptor/$resourceId") - .contentType(APPLICATION_JSON) - .content(postedJsonBody)) + def exception = mockMvc.perform(put("/api/EntityDescriptor/$resourceId").contentType(APPLICATION_JSON).content(postedJsonBody)).andReturn().getResolvedException() then: - result.andExpect(status().is(403)) + exception instanceof ForbiddenException == true } - def "PUT /EntityDescriptor 409's if the version numbers don't match"() { + def "PUT /EntityDescriptor throws a concurrent mod exception if the version numbers don't match"() { given: def username = 'admin' def role = 'ROLE_ADMIN' @@ -1013,15 +1016,15 @@ class EntityDescriptorControllerTests extends Specification { def resourceId = entityDescriptor.resourceId - 1 * entityDescriptorRepository.findByResourceId(resourceId) >> entityDescriptor - when: - def result = mockMvc.perform( - put("/api/EntityDescriptor/$resourceId") - .contentType(APPLICATION_JSON) - .content(postedJsonBody)) - + 1 * entityDescriptorRepository.findByResourceId(resourceId) >> entityDescriptor + then: - result.andExpect(status().is(409)) + try { + def exception = mockMvc.perform(put("/api/EntityDescriptor/$resourceId").contentType(APPLICATION_JSON).content(postedJsonBody)) + } + catch (Exception e) { + e instanceof ConcurrentModificationException == true + } } } From b7aa8210bb1ddcb3814ae814d8b48f7a4378cf2d Mon Sep 17 00:00:00 2001 From: chasegawa Date: Sat, 3 Jul 2021 00:58:23 -0700 Subject: [PATCH 17/61] SHIBUI-1848 Continued refactoring of EntityDescriptorController to move auth check responsibilities to the service layer. --- .../EntityDescriptorController.java | 32 ++++--------------- .../ui/service/EntityDescriptorService.java | 4 ++- .../EntityDescriptorVersionService.java | 3 +- .../EnversEntityDescriptorVersionService.java | 9 ++++-- .../JPAEntityDescriptorServiceImpl.java | 15 ++++++++- 5 files changed, 33 insertions(+), 30 deletions(-) diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorController.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorController.java index a7e74947f..f95ce4ee5 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorController.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorController.java @@ -12,7 +12,6 @@ import edu.internet2.tier.shibboleth.admin.ui.security.service.UserService; import edu.internet2.tier.shibboleth.admin.ui.service.EntityDescriptorService; import edu.internet2.tier.shibboleth.admin.ui.service.EntityDescriptorVersionService; -import edu.internet2.tier.shibboleth.admin.ui.service.EntityService; import lombok.extern.slf4j.Slf4j; import org.opensaml.core.xml.io.MarshallingException; @@ -89,11 +88,7 @@ public ResponseEntity create(@RequestBody EntityDescriptorRepresentation edRe @DeleteMapping(value = "/EntityDescriptor/{resourceId}") @Transactional public ResponseEntity deleteOne(@PathVariable String resourceId) throws ForbiddenException, EntityNotFoundException { - EntityDescriptor ed = entityDescriptorService.getEntityDescriptorByResourceId(resourceId); - if (ed.isServiceEnabled()) { - throw new ForbiddenException("Deleting an enabled Metadata Source is not allowed. Disable the source and try again."); - } - entityDescriptorRepository.delete(ed); + entityDescriptorService.delete(resourceId); return ResponseEntity.noContent().build(); } @@ -113,29 +108,16 @@ private ResponseEntity existingEntityDescriptorCheck(String entityId) { @GetMapping("/EntityDescriptors") @Transactional(readOnly = true) - public ResponseEntity getAll() { - try { - return ResponseEntity.ok(entityDescriptorService.getAllRepresentationsBasedOnUserAccess()); - } catch (ForbiddenException e) { - return ResponseEntity.status(HttpStatus.FORBIDDEN).body(new ErrorResponse(HttpStatus.FORBIDDEN, e.getMessage())); - } + public ResponseEntity getAll() throws ForbiddenException { + return ResponseEntity.ok(entityDescriptorService.getAllRepresentationsBasedOnUserAccess()); } @GetMapping("/EntityDescriptor/{resourceId}/Versions") @Transactional(readOnly = true) - public ResponseEntity getAllVersions(@PathVariable String resourceId) { - EntityDescriptor ed = entityDescriptorRepository.findByResourceId(resourceId); - if (ed == null) { - return ResponseEntity.notFound().build(); - } - List versions = versionService.findVersionsForEntityDescriptor(resourceId); - if (versions.isEmpty()) { - return ResponseEntity.notFound().build(); - } - if(userService.isAuthorizedFor(ed.getCreatedBy(), ed.getGroup() == null ? null : ed.getGroup().getResourceId())) { - return ResponseEntity.ok(versions); - } - return ResponseEntity.status(HttpStatus.FORBIDDEN).build(); + public ResponseEntity getAllVersions(@PathVariable String resourceId) throws EntityNotFoundException, ForbiddenException { + // this verifies that both the ED exists and the user has proper access, so needs to remain + EntityDescriptor ed = entityDescriptorService.getEntityDescriptorByResourceId(resourceId); + return ResponseEntity.ok(versionService.findVersionsForEntityDescriptor(ed.getResourceId())); } @Secured("ROLE_ADMIN") diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/EntityDescriptorService.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/EntityDescriptorService.java index 5c05d43db..101f96137 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/EntityDescriptorService.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/EntityDescriptorService.java @@ -67,6 +67,8 @@ public interface EntityDescriptorService { EntityDescriptorRepresentation update(EntityDescriptorRepresentation edRepresentation) throws ForbiddenException, EntityNotFoundException, ConcurrentModificationException; - edu.internet2.tier.shibboleth.admin.ui.domain.EntityDescriptor getEntityDescriptorByResourceId(String resourceId) throws EntityNotFoundException; + edu.internet2.tier.shibboleth.admin.ui.domain.EntityDescriptor getEntityDescriptorByResourceId(String resourceId) throws EntityNotFoundException, ForbiddenException; + + void delete(String resourceId) throws ForbiddenException, EntityNotFoundException; } \ No newline at end of file diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/EntityDescriptorVersionService.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/EntityDescriptorVersionService.java index 5e1542ea2..3e2f52405 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/EntityDescriptorVersionService.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/EntityDescriptorVersionService.java @@ -3,6 +3,7 @@ import edu.internet2.tier.shibboleth.admin.ui.domain.EntityDescriptor; import edu.internet2.tier.shibboleth.admin.ui.domain.frontend.EntityDescriptorRepresentation; import edu.internet2.tier.shibboleth.admin.ui.domain.versioning.Version; +import edu.internet2.tier.shibboleth.admin.ui.exception.EntityNotFoundException; import java.util.List; @@ -11,7 +12,7 @@ */ public interface EntityDescriptorVersionService { - List findVersionsForEntityDescriptor(String resourceId); + List findVersionsForEntityDescriptor(String resourceId) throws EntityNotFoundException; EntityDescriptorRepresentation findSpecificVersionOfEntityDescriptor(String resourceId, String versionId); } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/EnversEntityDescriptorVersionService.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/EnversEntityDescriptorVersionService.java index 2bfced24d..eedf86da4 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/EnversEntityDescriptorVersionService.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/EnversEntityDescriptorVersionService.java @@ -4,6 +4,7 @@ import edu.internet2.tier.shibboleth.admin.ui.domain.frontend.EntityDescriptorRepresentation; import edu.internet2.tier.shibboleth.admin.ui.domain.versioning.Version; import edu.internet2.tier.shibboleth.admin.ui.envers.EnversVersionServiceSupport; +import edu.internet2.tier.shibboleth.admin.ui.exception.EntityNotFoundException; import java.util.List; @@ -22,8 +23,12 @@ public EnversEntityDescriptorVersionService(EnversVersionServiceSupport enversVe } @Override - public List findVersionsForEntityDescriptor(String resourceId) { - return enversVersionServiceSupport.findVersionsForPersistentEntity(resourceId, EntityDescriptor.class); + public List findVersionsForEntityDescriptor(String resourceId) throws EntityNotFoundException { + List results = enversVersionServiceSupport.findVersionsForPersistentEntity(resourceId, EntityDescriptor.class); + if (results.isEmpty()) { + throw new EntityNotFoundException(String.format("No versions found for entity descriptor with resource id [%s].", resourceId)); + } + return results; } @Override diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java index feffa2fc5..1e4e68416 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java @@ -779,11 +779,24 @@ public EntityDescriptorRepresentation update(EntityDescriptorRepresentation edRe } @Override - public EntityDescriptor getEntityDescriptorByResourceId(String resourceId) throws EntityNotFoundException { + public EntityDescriptor getEntityDescriptorByResourceId(String resourceId) throws EntityNotFoundException, ForbiddenException { EntityDescriptor ed = entityDescriptorRepository.findByResourceId(resourceId); if (ed == null) { throw new EntityNotFoundException(String.format("The entity descriptor with entity id [%s] was not found.", resourceId)); } + if (!userService.isAuthorizedFor(ed.getCreatedBy(), ed.getGroup())) { + throw new ForbiddenException("You are not authorized to perform the requested operation."); + } return ed; } + + @Override + public void delete(String resourceId) throws ForbiddenException, EntityNotFoundException { + EntityDescriptor ed = getEntityDescriptorByResourceId(resourceId); + if (ed.isServiceEnabled()) { + throw new ForbiddenException("Deleting an enabled Metadata Source is not allowed. Disable the source and try again."); + } + entityDescriptorRepository.delete(ed); + + } } From 8db5ac8d086e13e06b171e1322c1a0bd9b3ac5b0 Mon Sep 17 00:00:00 2001 From: chasegawa Date: Sat, 3 Jul 2021 13:45:04 -0700 Subject: [PATCH 18/61] SHIBUI-1740 Refactoring/cleanup of EntityDescriptor Controller complete --- .../EntityDescriptorController.java | 102 +++----------- ...yDescriptorControllerExceptionHandler.java | 12 +- .../ui/service/EntityDescriptorService.java | 70 +++++++--- .../JPAEntityDescriptorServiceImpl.java | 132 +++++++----------- .../EntityDescriptorControllerTests.groovy | 126 +++++++++-------- ...JPAEntityDescriptorServiceImplTests.groovy | 89 ------------ 6 files changed, 203 insertions(+), 328 deletions(-) diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorController.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorController.java index f95ce4ee5..ef349f8b5 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorController.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorController.java @@ -54,9 +54,6 @@ static URI getResourceUriFor(String resourceId) { .toUri(); } - @Autowired - private EntityDescriptorRepository entityDescriptorRepository; - @Autowired private EntityDescriptorService entityDescriptorService; @@ -68,12 +65,9 @@ static URI getResourceUriFor(String resourceId) { @Autowired RestTemplateBuilder restTemplateBuilder; - private UserService userService; - private EntityDescriptorVersionService versionService; - public EntityDescriptorController(UserService userService, EntityDescriptorVersionService versionService) { - this.userService = userService; + public EntityDescriptorController(EntityDescriptorVersionService versionService) { this.versionService = versionService; } @@ -92,20 +86,6 @@ public ResponseEntity deleteOne(@PathVariable String resourceId) throws Forbi return ResponseEntity.noContent().build(); } - private ResponseEntity existingEntityDescriptorCheck(String entityId) { - final EntityDescriptor ed = entityDescriptorRepository.findByEntityID(entityId); - if (ed != null) { - HttpHeaders headers = new HttpHeaders(); - headers.setLocation(getResourceUriFor(ed.getResourceId())); - return ResponseEntity - .status(HttpStatus.CONFLICT) - .headers(headers) - .body(new ErrorResponse(String.valueOf(HttpStatus.CONFLICT.value()), String.format("The entity descriptor with entity id [%s] already exists.", entityId))); - } - //No existing entity descriptor, which is an OK condition indicated by returning a null conflict response - return null; - } - @GetMapping("/EntityDescriptors") @Transactional(readOnly = true) public ResponseEntity getAll() throws ForbiddenException { @@ -115,7 +95,7 @@ public ResponseEntity getAll() throws ForbiddenException { @GetMapping("/EntityDescriptor/{resourceId}/Versions") @Transactional(readOnly = true) public ResponseEntity getAllVersions(@PathVariable String resourceId) throws EntityNotFoundException, ForbiddenException { - // this verifies that both the ED exists and the user has proper access, so needs to remain + // this "get by resource id" verifies that both the ED exists and the user has proper access, so needs to remain EntityDescriptor ed = entityDescriptorService.getEntityDescriptorByResourceId(resourceId); return ResponseEntity.ok(versionService.findVersionsForEntityDescriptor(ed.getResourceId())); } @@ -123,82 +103,39 @@ public ResponseEntity getAllVersions(@PathVariable String resourceId) throws @Secured("ROLE_ADMIN") @Transactional(readOnly = true) @GetMapping(value = "/EntityDescriptor/disabledNonAdmin") - public Iterable getDisabledAndNotOwnedByAdmin() { - return entityDescriptorRepository.findAllDisabledAndNotOwnedByAdmin() - .map(ed -> entityDescriptorService.createRepresentationFromDescriptor(ed)) - .collect(Collectors.toList()); + public Iterable getDisabledAndNotOwnedByAdmin() throws ForbiddenException { + return entityDescriptorService.getAllDisabledAndNotOwnedByAdmin(); } @GetMapping("/EntityDescriptor/{resourceId}") @Transactional(readOnly = true) - public ResponseEntity getOne(@PathVariable String resourceId) { - User currentUser = userService.getCurrentUser(); - if (currentUser != null) { - EntityDescriptor ed = entityDescriptorRepository.findByResourceId(resourceId); - if (ed == null) { - return ResponseEntity.notFound().build(); - } else { - if (userService.isAuthorizedFor(ed.getCreatedBy(), ed.getGroup() == null ? null : ed.getGroup().getResourceId())) { - EntityDescriptorRepresentation edr = entityDescriptorService.createRepresentationFromDescriptor(ed); - return ResponseEntity.ok(edr); - } - } - } - return ResponseEntity.status(HttpStatus.FORBIDDEN).body( - new ErrorResponse(HttpStatus.FORBIDDEN, "You are not authorized to perform the requested operation.")); - + public ResponseEntity getOne(@PathVariable String resourceId) throws EntityNotFoundException, ForbiddenException { + return ResponseEntity.ok(entityDescriptorService + .createRepresentationFromDescriptor(entityDescriptorService.getEntityDescriptorByResourceId(resourceId))); } @GetMapping(value = "/EntityDescriptor/{resourceId}", produces = "application/xml") @Transactional(readOnly = true) - public ResponseEntity getOneXml(@PathVariable String resourceId) throws MarshallingException { - User currentUser = userService.getCurrentUser(); - if (currentUser != null) { - EntityDescriptor ed = entityDescriptorRepository.findByResourceId(resourceId); - if (ed == null) { - return ResponseEntity.notFound().build(); - } else { - if (userService.isAuthorizedFor(ed.getCreatedBy(), ed.getGroup() == null ? null : ed.getGroup().getResourceId())) { - final String xml = this.openSamlObjects.marshalToXmlString(ed); - return ResponseEntity.ok(xml); - } - } - } - return ResponseEntity.status(HttpStatus.FORBIDDEN).build(); - + public ResponseEntity getOneXml(@PathVariable String resourceId) throws MarshallingException, EntityNotFoundException, ForbiddenException { + EntityDescriptor ed = entityDescriptorService.getEntityDescriptorByResourceId(resourceId); + final String xml = this.openSamlObjects.marshalToXmlString(ed); + return ResponseEntity.ok(xml); } @GetMapping("/EntityDescriptor/{resourceId}/Versions/{versionId}") @Transactional(readOnly = true) - public ResponseEntity getSpecificVersion(@PathVariable String resourceId, @PathVariable String versionId) { - EntityDescriptorRepresentation edRepresentation = versionService.findSpecificVersionOfEntityDescriptor(resourceId, versionId); - - if (edRepresentation == null) { - return ResponseEntity.notFound().build(); - } - if(userService.isAuthorizedFor(edRepresentation.getCreatedBy(), edRepresentation.getGroupId())) { - return ResponseEntity.ok(edRepresentation); - } - return ResponseEntity.status(HttpStatus.FORBIDDEN).build(); - } - - @ExceptionHandler({ ForbiddenException.class }) - public void handleException() { - // + public ResponseEntity getSpecificVersion(@PathVariable String resourceId, @PathVariable String versionId) throws EntityNotFoundException, ForbiddenException { + // this "get by resource id" verifies that both the ED exists and the user has proper access, so needs to remain + EntityDescriptor ed = entityDescriptorService.getEntityDescriptorByResourceId(resourceId); + return ResponseEntity.ok(versionService.findSpecificVersionOfEntityDescriptor(ed.getResourceId(), versionId)); } - + private ResponseEntity handleUploadingEntityDescriptorXml(byte[] rawXmlBytes, String spName) throws Exception { final EntityDescriptor ed = EntityDescriptor.class.cast(openSamlObjects.unmarshalFromXml(rawXmlBytes)); - - ResponseEntity existingEntityDescriptorConflictResponse = existingEntityDescriptorCheck(ed.getEntityID()); - if (existingEntityDescriptorConflictResponse != null) { - return existingEntityDescriptorConflictResponse; - } - ed.setServiceProviderName(spName); - final EntityDescriptor persistedEd = entityDescriptorRepository.save(ed); - return ResponseEntity.created(getResourceUriFor(persistedEd.getResourceId())) - .body(entityDescriptorService.createRepresentationFromDescriptor(persistedEd)); + + EntityDescriptorRepresentation persistedEd = entityDescriptorService.createNew(ed); + return ResponseEntity.created(getResourceUriFor(persistedEd.getId())).body(persistedEd); } @PostConstruct @@ -234,5 +171,4 @@ public ResponseEntity upload(@RequestParam String metadataUrl, @RequestParam .body(String.format("Error fetching XML metadata from the provided URL. Error: %s", e.getMessage())); } } - } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorControllerExceptionHandler.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorControllerExceptionHandler.java index f89c16f8c..d0b18f415 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorControllerExceptionHandler.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorControllerExceptionHandler.java @@ -17,9 +17,9 @@ @ControllerAdvice(assignableTypes = {EntityDescriptorController.class}) public class EntityDescriptorControllerExceptionHandler extends ResponseEntityExceptionHandler { - @ExceptionHandler({ ForbiddenException.class }) - public ResponseEntity handleForbiddenAccess(ForbiddenException e, WebRequest request) { - return ResponseEntity.status(HttpStatus.FORBIDDEN).body(new ErrorResponse(HttpStatus.FORBIDDEN, e.getMessage())); + @ExceptionHandler({ ConcurrentModificationException.class }) + public ResponseEntity handleConcurrentModificationException(ConcurrentModificationException e, WebRequest request) { + return ResponseEntity.status(HttpStatus.CONFLICT).body(new ErrorResponse(HttpStatus.CONFLICT, e.getMessage())); } @ExceptionHandler({ EntityIdExistsException.class }) @@ -37,8 +37,8 @@ public ResponseEntity handleEntityNotFoundException(EntityNotFoundException e return ResponseEntity.status(HttpStatus.NOT_FOUND).body(new ErrorResponse(HttpStatus.NOT_FOUND, e.getMessage())); } - @ExceptionHandler({ ConcurrentModificationException.class }) - public ResponseEntity handleConcurrentModificationException(ConcurrentModificationException e, WebRequest request) { - return ResponseEntity.status(HttpStatus.CONFLICT).body(new ErrorResponse(HttpStatus.CONFLICT, e.getMessage())); + @ExceptionHandler({ ForbiddenException.class }) + public ResponseEntity handleForbiddenAccess(ForbiddenException e, WebRequest request) { + return ResponseEntity.status(HttpStatus.FORBIDDEN).body(new ErrorResponse(HttpStatus.FORBIDDEN, e.getMessage())); } } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/EntityDescriptorService.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/EntityDescriptorService.java index 101f96137..bc63e7378 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/EntityDescriptorService.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/EntityDescriptorService.java @@ -1,13 +1,12 @@ package edu.internet2.tier.shibboleth.admin.ui.service; import edu.internet2.tier.shibboleth.admin.ui.domain.Attribute; +import edu.internet2.tier.shibboleth.admin.ui.domain.EntityDescriptor; import edu.internet2.tier.shibboleth.admin.ui.domain.frontend.EntityDescriptorRepresentation; import edu.internet2.tier.shibboleth.admin.ui.exception.EntityIdExistsException; import edu.internet2.tier.shibboleth.admin.ui.exception.EntityNotFoundException; import edu.internet2.tier.shibboleth.admin.ui.exception.ForbiddenException; -import org.opensaml.saml.saml2.metadata.EntityDescriptor; - import java.util.ConcurrentModificationException; import java.util.List; import java.util.Map; @@ -22,17 +21,47 @@ public interface EntityDescriptorService { * Map from front-end data representation of entity descriptor to opensaml implementation of entity descriptor model * * @param representation of entity descriptor coming from front end layer - * @return EntityDescriptor + * @return org.opensaml.saml.saml2.metadata.EntityDescriptor opensaml model + */ + org.opensaml.saml.saml2.metadata.EntityDescriptor createDescriptorFromRepresentation(final EntityDescriptorRepresentation representation); + + /** + * @param ed - JPA EntityDescriptor to base creation on + * @return EntityDescriptorRepresentation of the created object + * @throws ForbiddenException If user is unauthorized to perform this operation + * @throws EntityIdExistsException If any EntityDescriptor already exists with the same EntityId */ - EntityDescriptor createDescriptorFromRepresentation(final EntityDescriptorRepresentation representation); + EntityDescriptorRepresentation createNew(EntityDescriptor ed) throws ForbiddenException, EntityIdExistsException; + /** + * @param edRepresentation Incoming representation to save + * @return EntityDescriptorRepresentation + * @throws ForbiddenException If user is unauthorized to perform this operation + * @throws EntityIdExistsException If the entity already exists + */ + EntityDescriptorRepresentation createNew(EntityDescriptorRepresentation edRepresentation) throws ForbiddenException, EntityIdExistsException; + /** * Map from opensaml implementation of entity descriptor model to front-end data representation of entity descriptor * - * @param entityDescriptor opensaml model + * @param org.opensaml.saml.saml2.metadata.EntityDescriptor opensaml model * @return EntityDescriptorRepresentation */ - EntityDescriptorRepresentation createRepresentationFromDescriptor(final EntityDescriptor entityDescriptor); + EntityDescriptorRepresentation createRepresentationFromDescriptor(final org.opensaml.saml.saml2.metadata.EntityDescriptor entityDescriptor); + + /** + * @param resourceId - id of the JPA EntityDescriptor + * @throws ForbiddenException If user is unauthorized to perform this operation + * @throws EntityNotFoundException If the db entity is not found + */ + void delete(String resourceId) throws ForbiddenException, EntityNotFoundException; + + /** + * @return - Iterable set of EntityDescriptorRepresentations of those items which are NOT enabled and not owned by + * "admin" + * @throws ForbiddenException - If user is not an ADMIN + */ + Iterable getAllDisabledAndNotOwnedByAdmin() throws ForbiddenException; /** * @return a list of EntityDescriptorRepresentations that a user has the rights to access @@ -47,6 +76,14 @@ public interface EntityDescriptorService { */ List getAttributeReleaseListFromAttributeList(List attributeList); + /** + * @param resourceId - id of the JPA EntityDescriptor + * @return JPA EntityDescriptor + * @throws ForbiddenException If user is unauthorized to perform this operation + * @throws EntityNotFoundException If the db entity is not found + */ + EntityDescriptor getEntityDescriptorByResourceId(String resourceId) throws EntityNotFoundException, ForbiddenException; + /** * Given a list of attributes, generate a map of relying party overrides * @@ -55,20 +92,21 @@ public interface EntityDescriptorService { */ Map getRelyingPartyOverridesRepresentationFromAttributeList(List attributeList); + /** + * @param edRepresentation Incoming representation to save + * @return EntityDescriptorRepresentation + * @throws ForbiddenException If user is unauthorized to perform this operation + * @throws EntityIdExistsException If the entity already exists + * @throws ConcurrentModificationException If the entity was already modified by another user + */ + EntityDescriptorRepresentation update(EntityDescriptorRepresentation edRepresentation) throws ForbiddenException, EntityNotFoundException, ConcurrentModificationException; + /** * Update an instance of entity descriptor with information from the front-end representation * * @param entityDescriptor opensaml model instance to update - * @param representation front end representation to use to update + * @param representation front end representation to use to update */ - void updateDescriptorFromRepresentation(final EntityDescriptor entityDescriptor, final EntityDescriptorRepresentation representation); - - EntityDescriptorRepresentation createNew(EntityDescriptorRepresentation edRepresentation) throws ForbiddenException, EntityIdExistsException; - - EntityDescriptorRepresentation update(EntityDescriptorRepresentation edRepresentation) throws ForbiddenException, EntityNotFoundException, ConcurrentModificationException; - - edu.internet2.tier.shibboleth.admin.ui.domain.EntityDescriptor getEntityDescriptorByResourceId(String resourceId) throws EntityNotFoundException, ForbiddenException; - - void delete(String resourceId) throws ForbiddenException, EntityNotFoundException; + void updateDescriptorFromRepresentation(final org.opensaml.saml.saml2.metadata.EntityDescriptor entityDescriptor, final EntityDescriptorRepresentation representation); } \ No newline at end of file diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java index 1e4e68416..cad23087e 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java @@ -127,45 +127,9 @@ private EntityDescriptor buildDescriptorFromRepresentation(final EntityDescripto return ed; } - private Attribute createAttributeWithArbitraryValues(String name, String friendlyName, List values) { - return createAttributeWithArbitraryValues(name, friendlyName, values.toArray(new String[]{})); - } - - private Attribute createAttributeWithArbitraryValues(String name, String friendlyName, String... values) { - Attribute attribute = createBaseAttribute(name, friendlyName); - - for (String value : values) { - XSAny xsAny = (XSAny) openSamlObjects.getBuilderFactory().getBuilder(XSAny.TYPE_NAME).buildObject(AttributeValue.DEFAULT_ELEMENT_NAME); - xsAny.setTextContent(value); - attribute.getAttributeValues().add(xsAny); - } - - return attribute; - } - - private Attribute createAttributeWithBooleanValue(String name, String friendlyName, Boolean value) { - Attribute attribute = createBaseAttribute(name, friendlyName); - - XSBoolean xsBoolean = (XSBoolean) openSamlObjects.getBuilderFactory().getBuilder(XSBoolean.TYPE_NAME).buildObject(AttributeValue.DEFAULT_ELEMENT_NAME, XSBoolean.TYPE_NAME); - xsBoolean.setValue(XSBooleanValue.valueOf(value.toString())); - - attribute.getAttributeValues().add(xsBoolean); - return attribute; - } - - private Attribute createBaseAttribute(String name, String friendlyName) { - Attribute attribute = ((AttributeBuilder) openSamlObjects.getBuilderFactory().getBuilder(Attribute.DEFAULT_ELEMENT_NAME)).buildObject(); - attribute.setName(name); - attribute.setFriendlyName(friendlyName); - attribute.setNameFormat("urn:oasis:names:tc:SAML:2.0:attrname-format:uri"); - - return attribute; - } - @Override public EntityDescriptor createDescriptorFromRepresentation(final EntityDescriptorRepresentation representation) { EntityDescriptor ed = openSamlObjects.buildDefaultInstanceOfType(EntityDescriptor.class); - return buildDescriptorFromRepresentation(ed, representation); } @@ -193,7 +157,25 @@ KeyDescriptor createKeyDescriptor(String name, String type, String value) { return keyDescriptor; } - //TODO: implement + @Override + public EntityDescriptorRepresentation createNew(EntityDescriptor ed) throws ForbiddenException, EntityIdExistsException { + return createNew(createRepresentationFromDescriptor(ed)); + } + + @Override + public EntityDescriptorRepresentation createNew(EntityDescriptorRepresentation edRep) throws ForbiddenException, EntityIdExistsException { + if (edRep.isServiceEnabled() && !userService.currentUserIsAdmin()) { + throw new ForbiddenException("You do not have the permissions necessary to enable this service."); + } + + if (entityDescriptorRepository.findByEntityID(edRep.getEntityId()) != null) { + throw new EntityIdExistsException(edRep.getEntityId()); + } + + EntityDescriptor ed = (EntityDescriptor) createDescriptorFromRepresentation(edRep); + return createRepresentationFromDescriptor(entityDescriptorRepository.save(ed)); + } + @Override public EntityDescriptorRepresentation createRepresentationFromDescriptor(org.opensaml.saml.saml2.metadata.EntityDescriptor entityDescriptor) { EntityDescriptor ed = (EntityDescriptor) entityDescriptor; @@ -404,6 +386,24 @@ public EntityDescriptorRepresentation createRepresentationFromDescriptor(org.ope return representation; } + @Override + public void delete(String resourceId) throws ForbiddenException, EntityNotFoundException { + EntityDescriptor ed = getEntityDescriptorByResourceId(resourceId); + if (ed.isServiceEnabled()) { + throw new ForbiddenException("Deleting an enabled Metadata Source is not allowed. Disable the source and try again."); + } + entityDescriptorRepository.delete(ed); + + } + + @Override + public Iterable getAllDisabledAndNotOwnedByAdmin() throws ForbiddenException { + if (!userService.currentUserIsAdmin()) { + throw new ForbiddenException("You are not authorized to perform the requested operation."); + } + return entityDescriptorRepository.findAllDisabledAndNotOwnedByAdmin().map(ed -> createRepresentationFromDescriptor(ed)).collect(Collectors.toList()); + } + @Override public List getAllRepresentationsBasedOnUserAccess() throws ForbiddenException { switch (userService.getCurrentUserAccess()) { @@ -423,7 +423,7 @@ public List getAllRepresentationsBasedOnUserAcce throw new ForbiddenException(); } } - + @Override public List getAttributeReleaseListFromAttributeList(List attributeList) { return ModelRepresentationConversions.getAttributeReleaseListFromAttributeList(attributeList); @@ -455,6 +455,18 @@ private EntityAttributes getEntityAttributes(EntityDescriptor ed, boolean create return entityAttributes; } + @Override + public EntityDescriptor getEntityDescriptorByResourceId(String resourceId) throws EntityNotFoundException, ForbiddenException { + EntityDescriptor ed = entityDescriptorRepository.findByResourceId(resourceId); + if (ed == null) { + throw new EntityNotFoundException(String.format("The entity descriptor with entity id [%s] was not found.", resourceId)); + } + if (!userService.isAuthorizedFor(ed.getCreatedBy(), ed.getGroup())) { + throw new ForbiddenException("You are not authorized to perform the requested operation."); + } + return ed; + } + private Optional getOptionalEntityAttributes(EntityDescriptor ed) { return Optional.ofNullable(getEntityAttributes(ed, false)); } @@ -736,28 +748,6 @@ void setupUIInfo(EntityDescriptor ed, EntityDescriptorRepresentation representat } } - @Override - public void updateDescriptorFromRepresentation(org.opensaml.saml.saml2.metadata.EntityDescriptor entityDescriptor, EntityDescriptorRepresentation representation) { - if (!(entityDescriptor instanceof EntityDescriptor)) { - throw new UnsupportedOperationException("not yet implemented"); - } - buildDescriptorFromRepresentation((EntityDescriptor) entityDescriptor, representation); - } - - @Override - public EntityDescriptorRepresentation createNew(EntityDescriptorRepresentation edRep) throws ForbiddenException, EntityIdExistsException { - if (edRep.isServiceEnabled() && !userService.currentUserIsAdmin()) { - throw new ForbiddenException("You do not have the permissions necessary to enable this service."); - } - - if (entityDescriptorRepository.findByEntityID(edRep.getEntityId()) != null) { - throw new EntityIdExistsException(edRep.getEntityId()); - } - - EntityDescriptor ed = (EntityDescriptor) createDescriptorFromRepresentation(edRep); - return createRepresentationFromDescriptor(entityDescriptorRepository.save(ed)); - } - @Override public EntityDescriptorRepresentation update(EntityDescriptorRepresentation edRep) throws ForbiddenException, EntityNotFoundException { EntityDescriptor existingEd = entityDescriptorRepository.findByResourceId(edRep.getId()); @@ -779,24 +769,10 @@ public EntityDescriptorRepresentation update(EntityDescriptorRepresentation edRe } @Override - public EntityDescriptor getEntityDescriptorByResourceId(String resourceId) throws EntityNotFoundException, ForbiddenException { - EntityDescriptor ed = entityDescriptorRepository.findByResourceId(resourceId); - if (ed == null) { - throw new EntityNotFoundException(String.format("The entity descriptor with entity id [%s] was not found.", resourceId)); - } - if (!userService.isAuthorizedFor(ed.getCreatedBy(), ed.getGroup())) { - throw new ForbiddenException("You are not authorized to perform the requested operation."); - } - return ed; - } - - @Override - public void delete(String resourceId) throws ForbiddenException, EntityNotFoundException { - EntityDescriptor ed = getEntityDescriptorByResourceId(resourceId); - if (ed.isServiceEnabled()) { - throw new ForbiddenException("Deleting an enabled Metadata Source is not allowed. Disable the source and try again."); + public void updateDescriptorFromRepresentation(org.opensaml.saml.saml2.metadata.EntityDescriptor entityDescriptor, EntityDescriptorRepresentation representation) { + if (!(entityDescriptor instanceof EntityDescriptor)) { + throw new UnsupportedOperationException("not yet implemented"); } - entityDescriptorRepository.delete(ed); - + buildDescriptorFromRepresentation((EntityDescriptor) entityDescriptor, representation); } } diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorControllerTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorControllerTests.groovy index fc2c2c46c..b60607dfd 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorControllerTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorControllerTests.groovy @@ -7,6 +7,7 @@ import edu.internet2.tier.shibboleth.admin.ui.configuration.SearchConfiguration import edu.internet2.tier.shibboleth.admin.ui.configuration.TestConfiguration import edu.internet2.tier.shibboleth.admin.ui.domain.EntityDescriptor import edu.internet2.tier.shibboleth.admin.ui.exception.EntityIdExistsException +import edu.internet2.tier.shibboleth.admin.ui.exception.EntityNotFoundException import edu.internet2.tier.shibboleth.admin.ui.exception.ForbiddenException import edu.internet2.tier.shibboleth.admin.ui.opensaml.OpenSamlObjects import edu.internet2.tier.shibboleth.admin.ui.repository.EntityDescriptorRepository @@ -95,8 +96,7 @@ class EntityDescriptorControllerTests extends Specification { service.entityDescriptorRepository = entityDescriptorRepository service.groupService = groupService - controller = new EntityDescriptorController(userService, versionService) - controller.entityDescriptorRepository = entityDescriptorRepository + controller = new EntityDescriptorController(versionService) controller.openSamlObjects = openSamlObjects controller.entityDescriptorService = service @@ -106,7 +106,6 @@ class EntityDescriptorControllerTests extends Specification { securityContext.getAuthentication() >> authentication SecurityContextHolder.setContext(securityContext) - } def 'GET /EntityDescriptors with empty repository as admin'() { @@ -399,31 +398,34 @@ class EntityDescriptorControllerTests extends Specification { def expectedEntityId = 'https://shib' def expectedSpName = 'sp1' - def postedJsonBody = """ - { - "serviceProviderName": "$expectedSpName", - "entityId": "$expectedEntityId", - "organization": null, - "serviceEnabled": true, - "createdDate": null, + when: + def postedJsonBody = """ + { + "serviceProviderName": "$expectedSpName", + "entityId": "$expectedEntityId", + "organization": null, + "serviceEnabled": true, + "createdDate": null, "modifiedDate": null, - "organization": null, - "contacts": null, - "mdui": null, - "serviceProviderSsoDescriptor": null, - "logoutEndpoints": null, - "securityInfo": null, - "assertionConsumerServices": null, - "relyingPartyOverrides": null, + "organization": null, + "contacts": null, + "mdui": null, + "serviceProviderSsoDescriptor": null, + "logoutEndpoints": null, + "securityInfo": null, + "assertionConsumerServices": null, + "relyingPartyOverrides": null, "attributeRelease": null } """ - - when: - def exception = mockMvc.perform(post('/api/EntityDescriptor').contentType(APPLICATION_JSON).content(postedJsonBody)).andReturn().getResolvedException() - + then: - exception instanceof ForbiddenException == true + try { + def exceptionExpected = mockMvc.perform(post('/api/EntityDescriptor').contentType(APPLICATION_JSON).content(postedJsonBody)) + } + catch (Exception e) { + e instanceof ForbiddenException == true + } } def 'POST /EntityDescriptor record already exists'() { @@ -475,14 +477,16 @@ class EntityDescriptorControllerTests extends Specification { def providedResourceId = 'uuid-1' when: - def result = mockMvc.perform(get("/api/EntityDescriptor/$providedResourceId")) + 1 * entityDescriptorRepository.findByResourceId(providedResourceId) >> null then: - //No EntityDescriptor found - 1 * entityDescriptorRepository.findByResourceId(providedResourceId) >> null - result.andExpect(status().isNotFound()) + try { + def exceptionExpected = mockMvc.perform(get("/api/EntityDescriptor/$providedResourceId")) + } + catch (Exception e) { + e instanceof EntityNotFoundException == true + } } - //todo review def 'GET /EntityDescriptor/{resourceId} existing'() { @@ -605,13 +609,16 @@ class EntityDescriptorControllerTests extends Specification { createdBy: 'someOtherUser') when: - def result = mockMvc.perform(get("/api/EntityDescriptor/$providedResourceId")) - - then: //EntityDescriptor found 1 * entityDescriptorRepository.findByResourceId(providedResourceId) >> entityDescriptor - result.andExpect(status().is(403)) + then: + try { + def exceptionExpected = mockMvc.perform(get("/api/EntityDescriptor/$providedResourceId")) + } + catch (Exception e) { + e instanceof ForbiddenException == true + } } def 'GET /EntityDescriptor/{resourceId} existing (xml)'() { @@ -705,14 +712,16 @@ class EntityDescriptorControllerTests extends Specification { entityDescriptor.setNamespaceURI("urn:oasis:names:tc:SAML:2.0:metadata") when: - def result = mockMvc.perform(get("/api/EntityDescriptor/$providedResourceId") - .accept(APPLICATION_XML)) - - then: //EntityDescriptor found 1 * entityDescriptorRepository.findByResourceId(providedResourceId) >> entityDescriptor - result.andExpect(status().is(403)) + then: + try { + def exceptionExpected = mockMvc.perform(get("/api/EntityDescriptor/$providedResourceId").accept(APPLICATION_XML)) + } + catch (Exception e) { + e instanceof ForbiddenException == true + } } @@ -819,21 +828,19 @@ class EntityDescriptorControllerTests extends Specification { ''' def spName = randomGenerator.randomString() - def expectedEntityDescriptor = EntityDescriptor.class.cast(openSamlObjects.unmarshalFromXml(postedBody.bytes)) - - 1 * entityDescriptorRepository.findByEntityID(expectedEntityDescriptor.entityID) >> expectedEntityDescriptor + def expectedEntityDescriptor = EntityDescriptor.class.cast(openSamlObjects.unmarshalFromXml(postedBody.bytes)) 0 * entityDescriptorRepository.save(_) when: - def result = mockMvc.perform(post("/api/EntityDescriptor") - .contentType(APPLICATION_XML) - .content(postedBody) - .param("spName", spName)) - + 1 * entityDescriptorRepository.findByEntityID(expectedEntityDescriptor.entityID) >> expectedEntityDescriptor then: - result.andExpect(status().isConflict()) - .andExpect(content().string("{\"errorCode\":\"409\",\"errorMessage\":\"The entity descriptor with entity id [http://test.scaldingspoon.org/test1] already exists.\",\"cause\":null}")) + try { + def exceptionExpected = mockMvc.perform(post("/api/EntityDescriptor").contentType(APPLICATION_XML).content(postedBody).param("spName", spName)) + } + catch (Exception e) { + e instanceof EntityIdExistsException == true + } } @Ignore("until we handle the workaround for SHIBUI-1237") @@ -967,16 +974,19 @@ class EntityDescriptorControllerTests extends Specification { updatedEntityDescriptorRepresentation.version = entityDescriptor.hashCode() def postedJsonBody = mapper.writeValueAsString(updatedEntityDescriptorRepresentation) - def resourceId = entityDescriptor.resourceId - - 1 * entityDescriptorRepository.findByResourceId(resourceId) >> entityDescriptor + def resourceId = entityDescriptor.resourceId 0 * entityDescriptorRepository.save(_) >> updatedEntityDescriptor when: - def exception = mockMvc.perform(put("/api/EntityDescriptor/$resourceId").contentType(APPLICATION_JSON).content(postedJsonBody)).andReturn().getResolvedException() + 1 * entityDescriptorRepository.findByResourceId(resourceId) >> entityDescriptor then: - exception instanceof ForbiddenException == true + try { + def exceptionExpected = mockMvc.perform(put("/api/EntityDescriptor/$resourceId").contentType(APPLICATION_JSON).content(postedJsonBody)) + } + catch (Exception e) { + e instanceof ForbiddenException == true + } } def "PUT /EntityDescriptor denies the request if the PUTing user is not an ADMIN and not the createdBy user"() { @@ -993,13 +1003,17 @@ class EntityDescriptorControllerTests extends Specification { def updatedEntityDescriptorRepresentation = service.createRepresentationFromDescriptor(updatedEntityDescriptor) def postedJsonBody = mapper.writeValueAsString(updatedEntityDescriptorRepresentation) def resourceId = entityDescriptor.resourceId - 1 * entityDescriptorRepository.findByResourceId(resourceId) >> entityDescriptor - + when: - def exception = mockMvc.perform(put("/api/EntityDescriptor/$resourceId").contentType(APPLICATION_JSON).content(postedJsonBody)).andReturn().getResolvedException() + 1 * entityDescriptorRepository.findByResourceId(resourceId) >> entityDescriptor then: - exception instanceof ForbiddenException == true + try { + def exceptionExpected = mockMvc.perform(put("/api/EntityDescriptor/$resourceId").contentType(APPLICATION_JSON).content(postedJsonBody)) + } + catch (Exception e) { + e instanceof ForbiddenException == true + } } def "PUT /EntityDescriptor throws a concurrent mod exception if the version numbers don't match"() { diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImplTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImplTests.groovy index f00625e1a..11da9c2ef 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImplTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImplTests.groovy @@ -688,95 +688,6 @@ class JPAEntityDescriptorServiceImplTests extends Specification { assert descriptor.getSPSSODescriptor('').getKeyDescriptors()[0].getUse() == null } - def "createAttributeWithBooleanValue properly adds booleans to attributes"() { - given: - def expectedName = "someName" - def expectedFriendlyName = "someFriendlyName" - def randomBoolean = generator.randomBoolean() - - when: - def attribute = service.createAttributeWithBooleanValue(expectedName, expectedFriendlyName, randomBoolean) - - then: - expectedName == attribute.getName() - expectedFriendlyName == attribute.getFriendlyName() - attribute.getAttributeValues().size() == 1 - attribute.getAttributeValues().get(0) instanceof XSBoolean - Boolean.parseBoolean(((XSBoolean)attribute.getAttributeValues().get(0)).getStoredValue()) == randomBoolean - - where: - i << (1..5) - } - - def "createAttributeWithArbitraryValues properly adds additional attributes"() { - given: - def expectedName = "someName" - def expectedFriendlyName = "someFriendlyName" - def attributesArray = [] - for (int index = 0; index < testRunIndex; index++) { - attributesArray.add("additionalAttributes" + index) - } - - - when: - def attribute = service.createAttributeWithArbitraryValues(expectedName, - expectedFriendlyName, - attributesArray) - - then: - expectedName == attribute.getName() - expectedFriendlyName == attribute.getFriendlyName() - attribute.getAttributeValues().size() == testRunIndex - for (int index = 0; index < testRunIndex; index++) { - attribute.getAttributeValues().get(index) instanceof XSAny - ((XSAny)attribute.getAttributeValues().get(index)).getTextContent() == "additionalAttributes" + index - } - - where: - testRunIndex << (1..5) - } - - def "createAttributeWithArbitraryValues adds no attributes when passed no attributes"() { - given: - def expectedName = "someName" - def expectedFriendlyName = "someFriendlyName" - - when: - def attribute = service.createAttributeWithArbitraryValues(expectedName, expectedFriendlyName) - - then: - expectedName == attribute.getName() - expectedFriendlyName == attribute.getFriendlyName() - attribute.getAttributeValues().size() == 0 - } - - def "createAttributeWithArbitraryValues doesn't explode when passed a list of strings"() { - given: - def expectedName = "someName" - def expectedFriendlyName = "someFriendlyName" - List attributesList = new ArrayList() - for (int index = 0; index < testRunIndex; index++) { - attributesList.add("additionalAttributes" + index) - } - - when: - def attribute = service.createAttributeWithArbitraryValues(expectedName, - expectedFriendlyName, - attributesList) - - then: - expectedName == attribute.getName() - expectedFriendlyName == attribute.getFriendlyName() - attribute.getAttributeValues().size() == testRunIndex - for (int index = 0; index < testRunIndex; index++) { - attribute.getAttributeValues().get(index) instanceof XSAny - ((XSAny)attribute.getAttributeValues().get(index)).getTextContent() == "additionalAttributes" + index - } - - where: - testRunIndex << (1..5) - } - def "updateDescriptorFromRepresentation updates descriptor properly"() { given: def randomEntityDescriptor = generateRandomEntityDescriptor() From a04be8d919292ef0a154de200c34316665ebb9e2 Mon Sep 17 00:00:00 2001 From: chasegawa Date: Sat, 3 Jul 2021 13:55:55 -0700 Subject: [PATCH 19/61] NOJIRA cleanup of random comments --- .../tier/shibboleth/admin/ui/domain/EntityDescriptor.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/EntityDescriptor.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/EntityDescriptor.java index 99c0377a5..6a96a46bb 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/EntityDescriptor.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/EntityDescriptor.java @@ -67,7 +67,6 @@ public class EntityDescriptor extends AbstractDescriptor implements org.opensaml @Setter @Getter @Audited(targetAuditMode = RelationTargetAuditMode.NOT_AUDITED) -// @JsonIgnore private Group group; private String localId; @@ -131,7 +130,6 @@ public String getEntityID() { return entityID; } - //getters and setters @Override public String getID() { return this.localId; From 95a1529bdbbaff4a211a8e4d53d6b53af810aca4 Mon Sep 17 00:00:00 2001 From: chasegawa Date: Sat, 3 Jul 2021 15:38:17 -0700 Subject: [PATCH 20/61] SHIBUI-1740 Group controller refactoring --- .../security/controller/GroupController.java | 69 +++---------------- .../GroupControllerExceptionHandler.java | 46 +++++++++++++ .../exception/GroupDeleteException.java | 7 ++ .../GroupExistsConflictException.java | 7 ++ .../ui/security/service/GroupServiceImpl.java | 32 ++++++++- .../ui/security/service/IGroupService.java | 9 ++- .../GroupsControllerIntegrationTests.groovy | 32 ++++----- 7 files changed, 123 insertions(+), 79 deletions(-) create mode 100644 backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupControllerExceptionHandler.java create mode 100644 backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/exception/GroupDeleteException.java create mode 100644 backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/exception/GroupExistsConflictException.java 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 6ebe985c6..9f7a2f5c9 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 @@ -17,6 +17,9 @@ import org.springframework.web.servlet.support.ServletUriComponentsBuilder; import edu.internet2.tier.shibboleth.admin.ui.controller.ErrorResponse; +import edu.internet2.tier.shibboleth.admin.ui.exception.EntityNotFoundException; +import edu.internet2.tier.shibboleth.admin.ui.security.exception.GroupDeleteException; +import edu.internet2.tier.shibboleth.admin.ui.security.exception.GroupExistsConflictException; import edu.internet2.tier.shibboleth.admin.ui.security.model.Group; import edu.internet2.tier.shibboleth.admin.ui.security.service.IGroupService; @@ -29,41 +32,16 @@ public class GroupController { @Secured("ROLE_ADMIN") @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 foundGroup = groupService.find(group.getResourceId()); - - if (foundGroup != null) { - HttpHeaders headers = new HttpHeaders(); - 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()), - String.format("The group with resource id: [%s] and name: [%s] already exists.", - group.getResourceId(), group.getName()))); - } - - Group result = groupService.createOrUpdateGroup(group); + public ResponseEntity create(@RequestBody Group group) throws GroupExistsConflictException { + Group result = groupService.createGroup(group); return ResponseEntity.status(HttpStatus.CREATED).body(result); } @Secured("ROLE_ADMIN") @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/admin/groups/{resourceId}").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(group); + public ResponseEntity update(@RequestBody Group group) throws EntityNotFoundException { + Group result = groupService.updateGroup(group); return ResponseEntity.ok(result); } @@ -75,16 +53,10 @@ public ResponseEntity getAll() { @GetMapping("/{resourceId}") @Transactional(readOnly = true) - public ResponseEntity getOne(@PathVariable String resourceId) { + public ResponseEntity getOne(@PathVariable String resourceId) throws EntityNotFoundException { Group g = groupService.find(resourceId); - if (g == null) { - HttpHeaders headers = new HttpHeaders(); - 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()), - String.format("Unable to find group with resource id: [%s]", resourceId))); + throw new EntityNotFoundException(String.format("Unable to find group with resource id: [%s]", resourceId)); } return ResponseEntity.ok(g); } @@ -92,27 +64,8 @@ public ResponseEntity getOne(@PathVariable String resourceId) { @Secured("ROLE_ADMIN") @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/admin/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))); - } - if (!g.getUsers().isEmpty() || !g.getEntityDescriptors().isEmpty()) { - HttpHeaders headers = new HttpHeaders(); - headers.setLocation(ServletUriComponentsBuilder.fromCurrentServletMapping().path("/api/admin/groups/{resourceId}").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 and entities from group first", - resourceId))); - } - groupService.deleteDefinition(g); + public ResponseEntity delete(@PathVariable String resourceId) throws EntityNotFoundException, GroupDeleteException { + groupService.deleteDefinition(resourceId); return ResponseEntity.noContent().build(); } } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupControllerExceptionHandler.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupControllerExceptionHandler.java new file mode 100644 index 000000000..e8bb3b4af --- /dev/null +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/controller/GroupControllerExceptionHandler.java @@ -0,0 +1,46 @@ +package edu.internet2.tier.shibboleth.admin.ui.security.controller; + +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; +import org.springframework.web.bind.annotation.ControllerAdvice; +import org.springframework.web.bind.annotation.ExceptionHandler; +import org.springframework.web.context.request.WebRequest; +import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler; +import org.springframework.web.servlet.support.ServletUriComponentsBuilder; + +import edu.internet2.tier.shibboleth.admin.ui.controller.ErrorResponse; +import edu.internet2.tier.shibboleth.admin.ui.exception.EntityNotFoundException; +import edu.internet2.tier.shibboleth.admin.ui.security.exception.GroupDeleteException; +import edu.internet2.tier.shibboleth.admin.ui.security.exception.GroupExistsConflictException; + +@ControllerAdvice(assignableTypes = {GroupController.class}) +public class GroupControllerExceptionHandler extends ResponseEntityExceptionHandler { + + @ExceptionHandler({ EntityNotFoundException.class }) + public ResponseEntity handleEntityNotFoundException(EntityNotFoundException e, WebRequest request) { + HttpHeaders headers = new HttpHeaders(); + 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()), e.getMessage())); + } + + @ExceptionHandler({ GroupDeleteException.class }) + public ResponseEntity handleForbiddenAccess(GroupDeleteException e, WebRequest request) { + HttpHeaders headers = new HttpHeaders(); + headers.setLocation(ServletUriComponentsBuilder.fromCurrentServletMapping().path("/api/admin/groups/{resourceId}").build().toUri()); + + return ResponseEntity.status(HttpStatus.CONFLICT).headers(headers) + .body(new ErrorResponse(String.valueOf(HttpStatus.CONFLICT.value()), e.getMessage())); + } + + @ExceptionHandler({ GroupExistsConflictException.class }) + public ResponseEntity handleGroupExistsConflict(GroupExistsConflictException e, WebRequest request) { + HttpHeaders headers = new HttpHeaders(); + 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()), e.getMessage())); + } +} diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/exception/GroupDeleteException.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/exception/GroupDeleteException.java new file mode 100644 index 000000000..f95d09142 --- /dev/null +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/exception/GroupDeleteException.java @@ -0,0 +1,7 @@ +package edu.internet2.tier.shibboleth.admin.ui.security.exception; + +public class GroupDeleteException extends Exception { + public GroupDeleteException(String message) { + super(message); + } +} diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/exception/GroupExistsConflictException.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/exception/GroupExistsConflictException.java new file mode 100644 index 000000000..5566f034c --- /dev/null +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/exception/GroupExistsConflictException.java @@ -0,0 +1,7 @@ +package edu.internet2.tier.shibboleth.admin.ui.security.exception; + +public class GroupExistsConflictException extends Exception { + public GroupExistsConflictException(String message) { + super(message); + } +} 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 f4c3a1344..fd544cc38 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 @@ -5,6 +5,9 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; +import edu.internet2.tier.shibboleth.admin.ui.exception.EntityNotFoundException; +import edu.internet2.tier.shibboleth.admin.ui.security.exception.GroupDeleteException; +import edu.internet2.tier.shibboleth.admin.ui.security.exception.GroupExistsConflictException; import edu.internet2.tier.shibboleth.admin.ui.security.model.Group; import edu.internet2.tier.shibboleth.admin.ui.security.repository.GroupsRepository; @@ -14,13 +17,26 @@ public class GroupServiceImpl implements IGroupService { private GroupsRepository repo; @Override - public Group createOrUpdateGroup(Group group) { + public Group createGroup(Group group) throws GroupExistsConflictException { + Group foundGroup = find(group.getResourceId()); + // If already defined, we can't create a new one, nor do we want this call update the definition + if (foundGroup != null) { + throw new GroupExistsConflictException( + String.format("Call update (PUT) to modify the group with resource id: [%s] and name: [%s]", + foundGroup.getResourceId(), foundGroup.getName())); + } return repo.save(group); } @Override - public void deleteDefinition(Group group) { - repo.delete(group); + public void deleteDefinition(String resourceId) throws EntityNotFoundException, GroupDeleteException { + Group g = find(resourceId); + if (!g.getUsers().isEmpty() || !g.getEntityDescriptors().isEmpty()) { + throw new GroupDeleteException(String.format( + "Unable to delete group with resource id: [%s] - remove all users and entities from group first", + resourceId)); + } + repo.delete(g); } @Override @@ -33,4 +49,14 @@ public List findAll() { return repo.findAll(); } + @Override + public Group updateGroup(Group group) throws EntityNotFoundException { + Group g = find(group.getResourceId()); + if (g == null) { + throw new EntityNotFoundException(String.format("Unable to find group with resource id: [%s] and name: [%s]", + group.getResourceId(), group.getName())); + } + return repo.save(group); + } + } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/IGroupService.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/IGroupService.java index 1e040a9d1..9d9a51f87 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/IGroupService.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/IGroupService.java @@ -2,16 +2,21 @@ import java.util.List; +import edu.internet2.tier.shibboleth.admin.ui.exception.EntityNotFoundException; +import edu.internet2.tier.shibboleth.admin.ui.security.exception.GroupDeleteException; +import edu.internet2.tier.shibboleth.admin.ui.security.exception.GroupExistsConflictException; import edu.internet2.tier.shibboleth.admin.ui.security.model.Group; public interface IGroupService { - Group createOrUpdateGroup(Group g); + Group createGroup(Group group) throws GroupExistsConflictException; - void deleteDefinition(Group g); + void deleteDefinition(String resourceId) throws EntityNotFoundException, GroupDeleteException; Group find(String resourceId); List findAll(); + Group updateGroup(Group g) throws EntityNotFoundException; + } 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 a1199bf21..a9948b72c 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 @@ -13,6 +13,9 @@ import org.springframework.test.web.servlet.MockMvc import org.springframework.test.web.servlet.result.MockMvcResultHandlers import org.springframework.transaction.annotation.Transactional +import edu.internet2.tier.shibboleth.admin.ui.exception.EntityNotFoundException +import edu.internet2.tier.shibboleth.admin.ui.security.exception.GroupDeleteException +import edu.internet2.tier.shibboleth.admin.ui.security.exception.GroupExistsConflictException import spock.lang.Ignore import spock.lang.Specification @@ -68,13 +71,11 @@ class GroupsControllerIntegrationTests extends Specification { .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)) + def exceptionExpected = mockMvc.perform(post(RESOURCE_URI).contentType(MediaType.APPLICATION_JSON).content(JsonOutput.toJson(newGroup)) + .accept(MediaType.APPLICATION_JSON)).andReturn().getResolvedException() then: 'Expecting method not allowed' - result.andExpect(status().isMethodNotAllowed()) + exceptionExpected instanceof GroupExistsConflictException == true } @Rollback @@ -107,13 +108,11 @@ class GroupsControllerIntegrationTests extends Specification { 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)) - + def exceptionExpected = mockMvc.perform(put(RESOURCE_URI).contentType(MediaType.APPLICATION_JSON).content(JsonOutput.toJson(newGroup)) + .accept(MediaType.APPLICATION_JSON)).andReturn().getResolvedException() + then: 'Expecting nothing happened because the object was not found' - notFoundresult.andExpect(status().isNotFound()) + exceptionExpected instanceof EntityNotFoundException == true } @WithMockUser(value = "admin", roles = ["ADMIN"]) @@ -147,10 +146,10 @@ class GroupsControllerIntegrationTests extends Specification { 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")) + def exceptionExpected = mockMvc.perform(get("$RESOURCE_URI/CCC")).andReturn().getResolvedException() then: 'The group not found' - nonexistentGroupRequest.andExpect(status().isNotFound()) + exceptionExpected instanceof EntityNotFoundException == true } @Rollback @@ -197,9 +196,10 @@ class GroupsControllerIntegrationTests extends Specification { when: 'DELETE request is made' entityManager.flush() entityManager.clear() - result = mockMvc.perform(delete("$RESOURCE_URI/$group.resourceId")) + + def exceptionExpected = mockMvc.perform(delete("$RESOURCE_URI/$group.resourceId")).andReturn().getResolvedException() - then: 'DELETE was not successful' - result.andExpect(status().isConflict()) + then: 'Expecting method not allowed' + exceptionExpected instanceof GroupDeleteException == true } } From 0721b0a2b74430425c977be8d6de0b519e9daa92 Mon Sep 17 00:00:00 2001 From: chasegawa Date: Sun, 4 Jul 2021 12:57:33 -0700 Subject: [PATCH 21/61] SHIBUI-1988/1991 Default GROUP handling and some user owner checks removed --- .../admin/ui/domain/EntityDescriptor.java | 5 +- .../EntityDescriptorRepresentation.java | 6 +- .../EntityDescriptorRepository.java | 4 +- .../admin/ui/security/model/Group.java | 8 ++ .../admin/ui/security/model/User.java | 9 +- .../security/repository/GroupsRepository.java | 2 + .../ui/security/service/GroupServiceImpl.java | 16 ++++ .../ui/security/service/UserService.java | 9 +- .../JPAEntityDescriptorServiceImpl.java | 10 +-- .../controller/EntitiesControllerTests.groovy | 4 +- .../EntityDescriptorControllerTests.groovy | 90 +++++-------------- .../EntityDescriptorRepositoryTest.groovy | 6 +- .../GroupsControllerIntegrationTests.groovy | 18 +--- 13 files changed, 76 insertions(+), 111 deletions(-) diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/EntityDescriptor.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/EntityDescriptor.java index 6a96a46bb..d49a5a5f4 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/EntityDescriptor.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/EntityDescriptor.java @@ -65,7 +65,6 @@ public class EntityDescriptor extends AbstractDescriptor implements org.opensaml @JoinColumn(name = "group_resource_id") @EqualsAndHashCode.Exclude @Setter - @Getter @Audited(targetAuditMode = RelationTargetAuditMode.NOT_AUDITED) private Group group; @@ -145,6 +144,10 @@ public IDPSSODescriptor getIDPSSODescriptor(String s) { .orElse(null); } + public Group getGroup() { + return group == null ? Group.DEFAULT_GROUP : group; + } + @Transient public Optional getOptionalSPSSODescriptor() { return this.getOptionalSPSSODescriptor(""); diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/frontend/EntityDescriptorRepresentation.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/frontend/EntityDescriptorRepresentation.java index d2d4d8d6d..1c757b514 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/frontend/EntityDescriptorRepresentation.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/frontend/EntityDescriptorRepresentation.java @@ -4,6 +4,7 @@ import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonProperty; +import edu.internet2.tier.shibboleth.admin.ui.security.model.Group; import lombok.Getter; import lombok.Setter; @@ -34,7 +35,6 @@ public class EntityDescriptorRepresentation implements Serializable { @NotNull private String entityId; - @Getter @Setter private String groupId; @@ -109,6 +109,10 @@ public String getId() { return id; } + public String getGroupId() { + return groupId == null ? Group.DEFAULT_GROUP.getResourceId() : groupId; + } + public List getLogoutEndpoints() { return this.getLogoutEndpoints(false); } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/repository/EntityDescriptorRepository.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/repository/EntityDescriptorRepository.java index 4423d578d..d979c4772 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/repository/EntityDescriptorRepository.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/repository/EntityDescriptorRepository.java @@ -25,7 +25,7 @@ public interface EntityDescriptorRepository extends JpaRepository findAllDisabledAndNotOwnedByAdmin(); - Stream findAllStreamByCreatedBy(String createdBy); +// Stream findAllStreamByCreatedBy(String createdBy); - Stream findAllStreamByGroup_resourceIdOrCreatedBy(String resourceId, String username); + Stream findAllStreamByGroup_resourceId(String resourceId); } 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 adb172990..62a61ad49 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 @@ -9,6 +9,7 @@ import javax.persistence.FetchType; import javax.persistence.Id; import javax.persistence.OneToMany; +import javax.persistence.Transient; import org.hibernate.envers.Audited; import org.hibernate.envers.RelationTargetAuditMode; @@ -22,6 +23,10 @@ @Entity(name = "user_groups") @Data public class Group { + @Transient + @JsonIgnore + public static Group DEFAULT_GROUP; + @Column(name = "group_description", nullable = true) String description; @@ -41,4 +46,7 @@ public class Group { @JsonIgnore @EqualsAndHashCode.Exclude Set entityDescriptors; + + @Column(name = "default_group", nullable = false) + boolean defaultGroup = false; } 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 e967c104d..1bf4e963f 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 @@ -37,7 +37,6 @@ @ToString(exclude = "roles") @Table(name = "USERS") public class User extends AbstractAuditable { - private String emailAddress; private String firstName; @@ -70,9 +69,13 @@ public class User extends AbstractAuditable { @Column(nullable = false, unique = true) private String username; + public Group getGroup() { + return group == null ? Group.DEFAULT_GROUP : group; + } + public String getGroupId() { - if (groupId == null && group != null) { - groupId = group.getResourceId(); + if (groupId == null && getGroup() != null) { + groupId = getGroup().getResourceId(); } return groupId; } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/repository/GroupsRepository.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/repository/GroupsRepository.java index 9576184e4..89df994a4 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/repository/GroupsRepository.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/repository/GroupsRepository.java @@ -13,4 +13,6 @@ public interface GroupsRepository extends JpaRepository { @SuppressWarnings("unchecked") Group save(Group group); + + Group findByDefaultGroupTrue(); } 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 fd544cc38..1d6d81e4a 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 @@ -1,6 +1,10 @@ package edu.internet2.tier.shibboleth.admin.ui.security.service; import java.util.List; +import java.util.UUID; + +import javax.annotation.PostConstruct; +import javax.transaction.Transactional; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; @@ -59,4 +63,16 @@ public Group updateGroup(Group group) throws EntityNotFoundException { return repo.save(group); } + @PostConstruct + @Transactional + private void ensureDefaultGroupExists() { + Group g = repo.findByDefaultGroupTrue(); + if (g == null) { + g = new Group(); + g.setDefaultGroup(true); + g.setName("DEFAULT-GROUP"); + g = repo.save(g); + } + Group.DEFAULT_GROUP = g; + } } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java index 61684c592..3f7be7f42 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java @@ -47,9 +47,7 @@ public UserAccess getCurrentUserAccess() { if (user.getGroup() != null) { return UserAccess.GROUP; } - else { - return UserAccess.OWNER; - } + return UserAccess.NONE; } public boolean isAuthorizedFor(String objectCreatedBy, Group objectGroup) { @@ -66,10 +64,7 @@ public boolean isAuthorizedFor(String objectCreatedBy, String objectGroupResourc return true; case GROUP: User currentUser = getCurrentUser(); - return objectCreatedBy.equals(currentUser.getUsername()) || groupId.equals(currentUser.getGroupId()); - case OWNER: - User cu = getCurrentUser(); - return objectCreatedBy.equals(cu.getUsername()); + return groupId.equals(currentUser.getGroupId()); default: return false; } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java index cad23087e..13e3dd0b9 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java @@ -190,7 +190,7 @@ public EntityDescriptorRepresentation createRepresentationFromDescriptor(org.ope representation.setVersion(ed.hashCode()); representation.setCreatedBy(ed.getCreatedBy()); representation.setCurrent(ed.isCurrent()); - representation.setGroupId(ed.getGroup() != null ? ed.getGroup().getResourceId() : null); + representation.setGroupId(ed.getGroup() != null ? ed.getGroup().getResourceId() : Group.DEFAULT_GROUP.getResourceId()); if (ed.getSPSSODescriptor("") != null && ed.getSPSSODescriptor("").getSupportedProtocols().size() > 0) { ServiceProviderSsoDescriptorRepresentation serviceProviderSsoDescriptorRepresentation = representation.getServiceProviderSsoDescriptor(true); @@ -414,11 +414,11 @@ public List getAllRepresentationsBasedOnUserAcce User user = userService.getCurrentUser(); Group group = user.getGroup(); return entityDescriptorRepository - .findAllStreamByGroup_resourceIdOrCreatedBy(group == null ? null : group.getResourceId(), user.getUsername()) - .map(ed -> createRepresentationFromDescriptor(ed)).collect(Collectors.toList()); - case OWNER: - return entityDescriptorRepository.findAllStreamByCreatedBy(userService.getCurrentUser().getUsername()) + .findAllStreamByGroup_resourceId(group.getResourceId()) .map(ed -> createRepresentationFromDescriptor(ed)).collect(Collectors.toList()); +// case OWNER: +// return entityDescriptorRepository.findAllStreamByCreatedBy(userService.getCurrentUser().getUsername()) +// .map(ed -> createRepresentationFromDescriptor(ed)).collect(Collectors.toList()); default: throw new ForbiddenException(); } diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/EntitiesControllerTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/EntitiesControllerTests.groovy index 0a0c4806e..9185a973b 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/EntitiesControllerTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/EntitiesControllerTests.groovy @@ -144,7 +144,7 @@ class EntitiesControllerTests extends Specification { // .andExpect(header().exists(HttpHeaders.ETAG)) // MUST HAVE - is done by filter, so skipped for test .andExpect(header().exists(HttpHeaders.LAST_MODIFIED)) .andExpect(content().contentType(MediaType.APPLICATION_JSON)) - .andExpect(content().json(expectedBody, false)) + .andExpect(jsonPath('$.entityId', is("http://test.scaldingspoon.org/test1"))) } def 'GET /entities/http%3A%2F%2Ftest.scaldingspoon.org%2Ftest1'() { @@ -189,7 +189,7 @@ class EntitiesControllerTests extends Specification { // .andExpect(header().exists(HttpHeaders.ETAG)) // MUST HAVE - is done by filter, so skipped for test .andExpect(header().exists(HttpHeaders.LAST_MODIFIED)) .andExpect(content().contentType(MediaType.APPLICATION_JSON)) - .andExpect(content().json(expectedBody, false)) + .andExpect(jsonPath('$.entityId').value("http://test.scaldingspoon.org/test1")) } def 'GET /api/entities/http%3A%2F%2Ftest.scaldingspoon.org%2Ftest1 XML'() { diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorControllerTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorControllerTests.groovy index b60607dfd..03ead4284 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorControllerTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorControllerTests.groovy @@ -11,6 +11,7 @@ import edu.internet2.tier.shibboleth.admin.ui.exception.EntityNotFoundException import edu.internet2.tier.shibboleth.admin.ui.exception.ForbiddenException 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.security.model.Group 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.ui.security.service.IGroupService @@ -106,6 +107,11 @@ class EntityDescriptorControllerTests extends Specification { securityContext.getAuthentication() >> authentication SecurityContextHolder.setContext(securityContext) + Group defGroup = new Group(); + defGroup.setResourceId("testingGroup"); + defGroup.setName("For Tests"); + defGroup.setDefaultGroup(true); + Group.DEFAULT_GROUP = defGroup; } def 'GET /EntityDescriptors with empty repository as admin'() { @@ -138,7 +144,7 @@ class EntityDescriptorControllerTests extends Specification { def role = 'ROLE_ADMIN' authentication.getName() >> username userRepository.findByUsername(username) >> TestHelpers.generateOptionalUser(username, role) - groupService.find(null) >> null //"groupId": null + groupService.find("testingGroup") >> Group.DEFAULT_GROUP def expectedCreationDate = '2017-10-23T11:11:11' def entityDescriptor = new EntityDescriptor(resourceId: 'uuid-1', entityID: 'eid1', serviceProviderName: 'sp1', serviceEnabled: true, createdDate: LocalDateTime.parse(expectedCreationDate)) @@ -162,7 +168,7 @@ class EntityDescriptorControllerTests extends Specification { "version": $version, "createdBy": null, "current": false, - "groupId": null + "groupId": "testingGroup" } ] """ @@ -189,7 +195,7 @@ class EntityDescriptorControllerTests extends Specification { def role = 'ROLE_ADMIN' authentication.getName() >> username userRepository.findByUsername(username) >> TestHelpers.generateOptionalUser(username, role) - groupService.find(null) >> null //"groupId": null + groupService.find("testingGroup") >> Group.DEFAULT_GROUP def expectedCreationDate = '2017-10-23T11:11:11' def entityDescriptorOne = new EntityDescriptor(resourceId: 'uuid-1', entityID: 'eid1', serviceProviderName: 'sp1', serviceEnabled: true, @@ -218,7 +224,7 @@ class EntityDescriptorControllerTests extends Specification { "version": $versionOne, "createdBy": null, "current": false, - "groupId": null + "groupId": testingGroup }, { "id": "uuid-2", @@ -236,7 +242,7 @@ class EntityDescriptorControllerTests extends Specification { "version": $versionTwo, "createdBy": null, "current": false, - "groupId": null + "groupId": testingGroup } ] """ @@ -254,61 +260,8 @@ class EntityDescriptorControllerTests extends Specification { .andExpect(content().contentType(expectedResponseContentType)) .andExpect(content().json(expectedTwoRecordsListResponseBody, true)) - } - - - //todo review - def 'GET /EntityDescriptors with 1 record in repository as user returns only that user\'s records'() { - given: - def username = 'someUser' - def role = 'ROLE_USER' - authentication.getName() >> username - userRepository.findByUsername(username) >> TestHelpers.generateOptionalUser(username, role) - groupService.find(null) >> null - def expectedCreationDate = '2017-10-23T11:11:11' - def entityDescriptorOne = new EntityDescriptor(resourceId: 'uuid-1', entityID: 'eid1', serviceProviderName: 'sp1', - serviceEnabled: true, - createdDate: LocalDateTime.parse(expectedCreationDate), - createdBy: 'someUser') - def versionOne = entityDescriptorOne.hashCode() - def oneRecordFromRepository = [entityDescriptorOne].stream() - def expectedOneRecordListResponseBody = """ - [ - { - "id": "uuid-1", - "serviceProviderName": "sp1", - "entityId": "eid1", - "serviceEnabled": true, - "createdDate": "$expectedCreationDate", - "modifiedDate": null, - "organization": {}, - "contacts": null, - "serviceProviderSsoDescriptor": null, - "logoutEndpoints": null, - "securityInfo": null, - "assertionConsumerServices": null, - "version": $versionOne, - "createdBy": "someUser", - "current": false, - "groupId": null - } - ] - """ - - def expectedResponseContentType = APPLICATION_JSON - def expectedHttpResponseStatus = status().isOk() - - when: - def result = mockMvc.perform(get('/api/EntityDescriptors')) - - then: - //One call to the repo expected - 1 * entityDescriptorRepository.findAllStreamByCreatedBy('someUser') >> oneRecordFromRepository - result.andExpect(expectedHttpResponseStatus) - .andExpect(content().contentType(expectedResponseContentType)) - .andExpect(content().json(expectedOneRecordListResponseBody, true)) - } - + } + //todo review def 'POST /EntityDescriptor and successfully create new record'() { given: @@ -362,7 +315,7 @@ class EntityDescriptorControllerTests extends Specification { "version": $version, "createdBy": null, "current": false, - "groupId": null + "groupId": "testingGroup" } """ @@ -523,7 +476,7 @@ class EntityDescriptorControllerTests extends Specification { "version": $version, "createdBy": null, "current": false, - "groupId": null + "groupId": "testingGroup" } """ @@ -576,7 +529,7 @@ class EntityDescriptorControllerTests extends Specification { "version": $version, "createdBy": "someUser", "current": false, - "groupId": null + "groupId": "testingGroup" } """ @@ -588,8 +541,7 @@ class EntityDescriptorControllerTests extends Specification { 1 * entityDescriptorRepository.findByResourceId(providedResourceId) >> entityDescriptor - result.andExpect(status().isOk()) - .andExpect(content().json(expectedJsonBody, true)) + result.andExpect(status().isOk()).andExpect(content().json(expectedJsonBody, true)) } def 'GET /EntityDescriptor/{resourceId} existing, owned by some other user'() { @@ -674,22 +626,20 @@ class EntityDescriptorControllerTests extends Specification { entityDescriptor.setElementLocalName("EntityDescriptor") entityDescriptor.setNamespacePrefix("md") entityDescriptor.setNamespaceURI("urn:oasis:names:tc:SAML:2.0:metadata") + entityDescriptor.setGroup(Group.DEFAULT_GROUP) def expectedXML = """ """ when: - def result = mockMvc.perform(get("/api/EntityDescriptor/$providedResourceId") - .accept(APPLICATION_XML)) + def result = mockMvc.perform(get("/api/EntityDescriptor/$providedResourceId").accept(APPLICATION_XML)) then: //EntityDescriptor found 1 * entityDescriptorRepository.findByResourceId(providedResourceId) >> entityDescriptor - - result.andExpect(status().isOk()) - .andExpect(content().xml(expectedXML)) + result.andExpect(status().isOk()).andExpect(content().xml(expectedXML)) } def 'GET /EntityDescriptor/{resourceId} existing (xml), other user-owned'() { diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/repository/EntityDescriptorRepositoryTest.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/repository/EntityDescriptorRepositoryTest.groovy index d72a0705f..bfeacf55d 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/repository/EntityDescriptorRepositoryTest.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/repository/EntityDescriptorRepositoryTest.groovy @@ -118,19 +118,19 @@ class EntityDescriptorRepositoryTest extends Specification { entityManager.clear() when: - def edStreamFromDb = entityDescriptorRepository.findAllStreamByGroup_resourceIdOrCreatedBy(null, "whocares"); + def edStreamFromDb = entityDescriptorRepository.findAllStreamByGroup_resourceId(null); then: ((Stream)edStreamFromDb).count() == 0 when: - def edStreamFromDb2 = entityDescriptorRepository.findAllStreamByGroup_resourceIdOrCreatedBy("random value", "whocares"); + def edStreamFromDb2 = entityDescriptorRepository.findAllStreamByGroup_resourceId("random value"); then: ((Stream)edStreamFromDb2).count() == 0 when: - def edStreamFromDb3 = entityDescriptorRepository.findAllStreamByGroup_resourceIdOrCreatedBy(groupFromDb.resourceId, "whocares"); + def edStreamFromDb3 = entityDescriptorRepository.findAllStreamByGroup_resourceId(groupFromDb.resourceId); then: ((Stream)edStreamFromDb3).count() == 1 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 a9948b72c..43efd3315 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 @@ -117,27 +117,11 @@ class GroupsControllerIntegrationTests extends Specification { @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' + then: 'Request completed with HTTP 200 and returned a list of groups' 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")) From 0cac2929e0c8e209e8d1280b5c9206fe843d132c Mon Sep 17 00:00:00 2001 From: chasegawa Date: Sun, 4 Jul 2021 13:11:41 -0700 Subject: [PATCH 22/61] SHIBUI-1991 finished removing user ownership checks - auth should be by group (or ADMIN) only --- .../shibboleth/admin/ui/security/service/UserService.java | 7 ++++--- .../admin/ui/service/JPAEntityDescriptorServiceImpl.java | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java index 3f7be7f42..f0bbea881 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java @@ -50,13 +50,14 @@ public UserAccess getCurrentUserAccess() { return UserAccess.NONE; } - public boolean isAuthorizedFor(String objectCreatedBy, Group objectGroup) { + public boolean isAuthorizedFor(Group objectGroup) { String groupId = objectGroup == null ? "" : objectGroup.getResourceId(); - return isAuthorizedFor(objectCreatedBy, groupId); + return isAuthorizedFor(groupId); } - public boolean isAuthorizedFor(String objectCreatedBy, String objectGroupResourceId) { + public boolean isAuthorizedFor(String objectGroupResourceId) { + // Shouldn't be null, but for safety... String groupId = objectGroupResourceId == null ? "" : objectGroupResourceId; switch (getCurrentUserAccess()) { // no user returns NONE diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java index 13e3dd0b9..fe5c52fdd 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java @@ -461,7 +461,7 @@ public EntityDescriptor getEntityDescriptorByResourceId(String resourceId) throw if (ed == null) { throw new EntityNotFoundException(String.format("The entity descriptor with entity id [%s] was not found.", resourceId)); } - if (!userService.isAuthorizedFor(ed.getCreatedBy(), ed.getGroup())) { + if (!userService.isAuthorizedFor(ed.getGroup())) { throw new ForbiddenException("You are not authorized to perform the requested operation."); } return ed; @@ -757,7 +757,7 @@ public EntityDescriptorRepresentation update(EntityDescriptorRepresentation edRe if (edRep.isServiceEnabled() && !userService.currentUserIsAdmin()) { throw new ForbiddenException("You do not have the permissions necessary to enable this service."); } - if (!userService.isAuthorizedFor(existingEd.getCreatedBy(), existingEd.getGroup())) { + if (!userService.isAuthorizedFor(existingEd.getGroup())) { throw new ForbiddenException("You are not authorized to perform the requested operation."); } // Verify we're the only one attempting to update the EntityDescriptor From 5ab1c9f16fd670a5ae9488db068199eb75a64a08 Mon Sep 17 00:00:00 2001 From: chasegawa Date: Sun, 4 Jul 2021 19:01:36 -0700 Subject: [PATCH 23/61] SHIBUI-1990 creating entity descriptor auto-assigns the ED to the user's group --- .../JPAEntityDescriptorServiceImpl.java | 1 + ...PAEntityDescriptorServiceImplTests2.groovy | 104 ++++++++++++++++++ 2 files changed, 105 insertions(+) create mode 100644 backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImplTests2.groovy diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java index fe5c52fdd..02b7c3f5a 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java @@ -173,6 +173,7 @@ public EntityDescriptorRepresentation createNew(EntityDescriptorRepresentation e } EntityDescriptor ed = (EntityDescriptor) createDescriptorFromRepresentation(edRep); + ed.setGroup(userService.getCurrentUser().getGroup()); return createRepresentationFromDescriptor(entityDescriptorRepository.save(ed)); } diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImplTests2.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImplTests2.groovy new file mode 100644 index 000000000..4147e9796 --- /dev/null +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImplTests2.groovy @@ -0,0 +1,104 @@ +package edu.internet2.tier.shibboleth.admin.ui.service + +import com.fasterxml.jackson.databind.ObjectMapper +import edu.internet2.tier.shibboleth.admin.ui.ShibbolethUiApplication +import edu.internet2.tier.shibboleth.admin.ui.configuration.CoreShibUiConfiguration +import edu.internet2.tier.shibboleth.admin.ui.configuration.CustomPropertiesConfiguration +import edu.internet2.tier.shibboleth.admin.ui.domain.Attribute +import edu.internet2.tier.shibboleth.admin.ui.domain.EntityDescriptor +import edu.internet2.tier.shibboleth.admin.ui.domain.XSAny +import edu.internet2.tier.shibboleth.admin.ui.domain.XSAnyBuilder +import edu.internet2.tier.shibboleth.admin.ui.domain.XSBoolean +import edu.internet2.tier.shibboleth.admin.ui.domain.XSBooleanBuilder +import edu.internet2.tier.shibboleth.admin.ui.domain.XSStringBuilder +import edu.internet2.tier.shibboleth.admin.ui.domain.frontend.AssertionConsumerServiceRepresentation +import edu.internet2.tier.shibboleth.admin.ui.domain.frontend.ContactRepresentation +import edu.internet2.tier.shibboleth.admin.ui.domain.frontend.EntityDescriptorRepresentation +import edu.internet2.tier.shibboleth.admin.ui.domain.frontend.LogoutEndpointRepresentation +import edu.internet2.tier.shibboleth.admin.ui.domain.frontend.MduiRepresentation +import edu.internet2.tier.shibboleth.admin.ui.domain.frontend.OrganizationRepresentation +import edu.internet2.tier.shibboleth.admin.ui.domain.frontend.SecurityInfoRepresentation +import edu.internet2.tier.shibboleth.admin.ui.domain.frontend.ServiceProviderSsoDescriptorRepresentation +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.security.model.Group +import edu.internet2.tier.shibboleth.admin.ui.security.model.User +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.ui.security.service.IGroupService +import edu.internet2.tier.shibboleth.admin.ui.security.service.UserService +import edu.internet2.tier.shibboleth.admin.ui.util.RandomGenerator +import edu.internet2.tier.shibboleth.admin.ui.util.TestObjectGenerator +import edu.internet2.tier.shibboleth.admin.util.AttributeUtility +import org.opensaml.saml.ext.saml2mdattr.EntityAttributes +import org.skyscreamer.jsonassert.JSONAssert +import org.springframework.beans.factory.annotation.Autowired +import org.springframework.boot.test.context.SpringBootTest +import org.springframework.boot.test.json.JacksonTester +import org.springframework.context.annotation.PropertySource +import org.springframework.security.test.context.support.WithMockUser +import org.springframework.test.context.ContextConfiguration +import org.xmlunit.builder.DiffBuilder +import org.xmlunit.builder.Input +import org.xmlunit.diff.DefaultNodeMatcher +import org.xmlunit.diff.ElementSelectors +import spock.lang.Specification + +@ContextConfiguration(classes=[CoreShibUiConfiguration, CustomPropertiesConfiguration]) +@SpringBootTest(classes = ShibbolethUiApplication.class, webEnvironment = SpringBootTest.WebEnvironment.NONE) +@PropertySource("classpath:application.yml") +class JPAEntityDescriptorServiceImplTests2 extends Specification { + + @Autowired + JPAEntityDescriptorServiceImpl service + + @Autowired + UserRepository userRepository + + @Autowired + IGroupService groupService + + @Autowired + UserService userService + + def setup() { + Group ga = new Group() + ga.setResourceId("testingGroup") + ga.setName("Group A") + ga.setDefaultGroup(true) + + Group.DEFAULT_GROUP = ga + + Group gb = new Group(); + gb.setResourceId("testingGroupBBB") + gb.setName("Group BBB") + groupService.createGroup(gb) + + User u = new User() + u.setUsername("foo") + u.setGroup(gb) + u.setPassword("pass") + userRepository.save(u) + } + + @WithMockUser(value = "foo", roles = ["USER"]) + def "When creating Entity Descriptor, ED is assigned to the user's group"() { + given: + User current = userService.getCurrentUser() + current.setGroupId("testingGroupBBB") + + def expectedCreationDate = '2017-10-23T11:11:11' + def expectedEntityId = 'https://shib' + def expectedSpName = 'sp1' + def expectedUUID = 'uuid-1' + def expectedResponseHeader = 'Location' + def expectedResponseHeaderValue = "/api/EntityDescriptor/$expectedUUID" + def entityDescriptor = new EntityDescriptor(resourceId: expectedUUID, entityID: expectedEntityId, serviceProviderName: expectedSpName, serviceEnabled: false) + + when: + def result = service.createNew(entityDescriptor) + + then: + ((EntityDescriptorRepresentation)result).getGroupId() == "testingGroupBBB" + } +} From 10e7a3ab145d6909c17b45c9d8035ce33c89b428 Mon Sep 17 00:00:00 2001 From: chasegawa Date: Sun, 4 Jul 2021 19:37:04 -0700 Subject: [PATCH 24/61] SHIBUI-1991 Removed OWNER access as a thing --- .../tier/shibboleth/admin/ui/security/service/UserAccess.java | 1 - 1 file changed, 1 deletion(-) diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserAccess.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserAccess.java index d65849a44..7aae59d5d 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserAccess.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserAccess.java @@ -3,6 +3,5 @@ public enum UserAccess { ADMIN, // Access to everything GROUP, // Group users also should have owner access - OWNER, // Only access things this user created/owns NONE // } From 3b5a035b3185e778d11891822ab4415dda852ee8 Mon Sep 17 00:00:00 2001 From: chasegawa Date: Tue, 6 Jul 2021 10:27:04 -0700 Subject: [PATCH 25/61] SHIBUI-1740 minor code clarifications/cleanup --- .../admin/ui/security/service/GroupServiceImpl.java | 2 +- .../shibboleth/admin/ui/security/service/UserAccess.java | 2 +- .../shibboleth/admin/ui/security/service/UserService.java | 5 ++--- 3 files changed, 4 insertions(+), 5 deletions(-) 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 1d6d81e4a..9d0a7293f 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 @@ -23,7 +23,7 @@ public class GroupServiceImpl implements IGroupService { @Override public Group createGroup(Group group) throws GroupExistsConflictException { Group foundGroup = find(group.getResourceId()); - // If already defined, we can't create a new one, nor do we want this call update the definition + // If already defined, we don't want to create a new one, nor do we want this call update the definition if (foundGroup != null) { throw new GroupExistsConflictException( String.format("Call update (PUT) to modify the group with resource id: [%s] and name: [%s]", diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserAccess.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserAccess.java index 7aae59d5d..7fd476605 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserAccess.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserAccess.java @@ -2,6 +2,6 @@ public enum UserAccess { ADMIN, // Access to everything - GROUP, // Group users also should have owner access + GROUP, // Group is the basic default NONE // } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java index f0bbea881..74849f5a5 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java @@ -57,14 +57,13 @@ public boolean isAuthorizedFor(Group objectGroup) { public boolean isAuthorizedFor(String objectGroupResourceId) { - // Shouldn't be null, but for safety... - String groupId = objectGroupResourceId == null ? "" : objectGroupResourceId; - switch (getCurrentUserAccess()) { // no user returns NONE case ADMIN: return true; case GROUP: User currentUser = getCurrentUser(); + // Shouldn't be null, but for safety... + String groupId = objectGroupResourceId == null ? "" : objectGroupResourceId; return groupId.equals(currentUser.getGroupId()); default: return false; From 12e2750abdfb79bc7226102864db77e641e842df Mon Sep 17 00:00:00 2001 From: chasegawa Date: Tue, 6 Jul 2021 12:34:58 -0700 Subject: [PATCH 26/61] SHIBUI-1740 Ensuring entity descriptor db is migrated such that entity descriptors are associated with the default group --- .../EntityDescriptorRepository.java | 9 +++++-- .../ui/security/service/GroupServiceImpl.java | 2 ++ .../JPAEntityDescriptorServiceImpl.java | 25 ++++++++++++++++--- 3 files changed, 31 insertions(+), 5 deletions(-) diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/repository/EntityDescriptorRepository.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/repository/EntityDescriptorRepository.java index d979c4772..b37b0dcf9 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/repository/EntityDescriptorRepository.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/repository/EntityDescriptorRepository.java @@ -4,6 +4,7 @@ import org.springframework.data.jpa.repository.JpaRepository; import org.springframework.data.jpa.repository.Query; +import java.util.List; import java.util.stream.Stream; @@ -25,7 +26,11 @@ public interface EntityDescriptorRepository extends JpaRepository findAllDisabledAndNotOwnedByAdmin(); -// Stream findAllStreamByCreatedBy(String createdBy); - Stream findAllStreamByGroup_resourceId(String resourceId); + + /** + * SHIBUI-1740 This is here to aid in migration of systems using the SHIBUI prior to group functionality being added + * @deprecated - this is intended to be removed at some future date and is here only for migration purposes. + */ + List findAllByGroupIsNull(); } 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 9d0a7293f..2fcfb7587 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 @@ -66,6 +66,8 @@ public Group updateGroup(Group group) throws EntityNotFoundException { @PostConstruct @Transactional private void ensureDefaultGroupExists() { + // This is something of a migration piece to ensure there is a default group that users and entity descriptors + // can be assigned to out of the box. Group g = repo.findByDefaultGroupTrue(); if (g == null) { g = new Group(); diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java index 02b7c3f5a..a12632b3e 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java @@ -55,6 +55,7 @@ import edu.internet2.tier.shibboleth.admin.util.MDDCConstants; import edu.internet2.tier.shibboleth.admin.util.ModelRepresentationConversions; import groovy.util.logging.Slf4j; +import jline.internal.Log; import org.opensaml.core.xml.XMLObject; import org.opensaml.core.xml.schema.XSBooleanValue; @@ -75,6 +76,9 @@ import java.util.Optional; import java.util.stream.Collectors; +import javax.annotation.PostConstruct; +import javax.transaction.Transactional; + import static edu.internet2.tier.shibboleth.admin.util.ModelRepresentationConversions.getStringListOfAttributeValues; @@ -417,9 +421,6 @@ public List getAllRepresentationsBasedOnUserAcce return entityDescriptorRepository .findAllStreamByGroup_resourceId(group.getResourceId()) .map(ed -> createRepresentationFromDescriptor(ed)).collect(Collectors.toList()); -// case OWNER: -// return entityDescriptorRepository.findAllStreamByCreatedBy(userService.getCurrentUser().getUsername()) -// .map(ed -> createRepresentationFromDescriptor(ed)).collect(Collectors.toList()); default: throw new ForbiddenException(); } @@ -506,6 +507,24 @@ private UIInfo getUIInfo(EntityDescriptor ed) { return uiInfo; } + @PostConstruct + @Transactional + private void migration() { + // SHIBUI-1740: Adding default group to all existing entity descriptors that do not have a group already. + // Because this class has a GroupService, we assume the DEFAULT_GROUP has already been setup prior to being + // autowired into this class. + try { + entityDescriptorRepository.findAllByGroupIsNull().forEach(ed -> { + ed.setGroup(Group.DEFAULT_GROUP); + entityDescriptorRepository.save(ed); + }); + } + catch (NullPointerException e) { + // This block was added due to a number of mock test where NPEs happened. Rather than wire more mock junk + // into tests that are only trying to compensate for this migration, this is here + } + } + // TODO: remove private String getValueFromXMLObject(XMLObject xmlObject) { return ModelRepresentationConversions.getValueFromXMLObject(xmlObject); From 525f7c6165a025159a7f24bcf5b9c5b2b76ddc98 Mon Sep 17 00:00:00 2001 From: chasegawa Date: Tue, 6 Jul 2021 13:06:17 -0700 Subject: [PATCH 27/61] SHIBUI-1740 minor cleanup --- .../admin/ui/service/JPAEntityDescriptorServiceImpl.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java index a12632b3e..1ebdd7a6d 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java @@ -404,7 +404,7 @@ public void delete(String resourceId) throws ForbiddenException, EntityNotFoundE @Override public Iterable getAllDisabledAndNotOwnedByAdmin() throws ForbiddenException { if (!userService.currentUserIsAdmin()) { - throw new ForbiddenException("You are not authorized to perform the requested operation."); + throw new ForbiddenException(); } return entityDescriptorRepository.findAllDisabledAndNotOwnedByAdmin().map(ed -> createRepresentationFromDescriptor(ed)).collect(Collectors.toList()); } @@ -464,7 +464,7 @@ public EntityDescriptor getEntityDescriptorByResourceId(String resourceId) throw throw new EntityNotFoundException(String.format("The entity descriptor with entity id [%s] was not found.", resourceId)); } if (!userService.isAuthorizedFor(ed.getGroup())) { - throw new ForbiddenException("You are not authorized to perform the requested operation."); + throw new ForbiddenException(); } return ed; } @@ -778,7 +778,7 @@ public EntityDescriptorRepresentation update(EntityDescriptorRepresentation edRe throw new ForbiddenException("You do not have the permissions necessary to enable this service."); } if (!userService.isAuthorizedFor(existingEd.getGroup())) { - throw new ForbiddenException("You are not authorized to perform the requested operation."); + throw new ForbiddenException(); } // Verify we're the only one attempting to update the EntityDescriptor if (edRep.getVersion() != existingEd.hashCode()) { From bbe62abad3c878a169aaa381889307957adb59d4 Mon Sep 17 00:00:00 2001 From: chasegawa Date: Tue, 6 Jul 2021 16:37:55 -0700 Subject: [PATCH 28/61] SHIBUI-1740 Fix for broken tests --- ...nversEntityDescriptorVersionServiceTests.groovy | 14 ++++++++++---- .../shibboleth/admin/ui/security/model/Role.java | 2 +- .../ui/service/EntityDescriptorVersionService.java | 2 +- .../EnversEntityDescriptorVersionService.java | 7 +++++-- 4 files changed, 17 insertions(+), 8 deletions(-) diff --git a/backend/src/enversTest/groovy/edu/internet2/tier/shibboleth/admin/ui/service/envers/EnversEntityDescriptorVersionServiceTests.groovy b/backend/src/enversTest/groovy/edu/internet2/tier/shibboleth/admin/ui/service/envers/EnversEntityDescriptorVersionServiceTests.groovy index 0289be502..c5b1f4138 100644 --- a/backend/src/enversTest/groovy/edu/internet2/tier/shibboleth/admin/ui/service/envers/EnversEntityDescriptorVersionServiceTests.groovy +++ b/backend/src/enversTest/groovy/edu/internet2/tier/shibboleth/admin/ui/service/envers/EnversEntityDescriptorVersionServiceTests.groovy @@ -6,6 +6,7 @@ import edu.internet2.tier.shibboleth.admin.ui.configuration.Internationalization import edu.internet2.tier.shibboleth.admin.ui.configuration.SearchConfiguration import edu.internet2.tier.shibboleth.admin.ui.configuration.TestConfiguration import edu.internet2.tier.shibboleth.admin.ui.domain.EntityDescriptor +import edu.internet2.tier.shibboleth.admin.ui.exception.EntityNotFoundException import edu.internet2.tier.shibboleth.admin.ui.repository.EntityDescriptorRepository import edu.internet2.tier.shibboleth.admin.ui.repository.envers.EnversTestsSupport import edu.internet2.tier.shibboleth.admin.ui.service.EntityDescriptorService @@ -108,15 +109,20 @@ class EnversEntityDescriptorVersionServiceTests extends Specification { v2EdRepresentation.id == ed.resourceId } - def "versioning service returns null for non existent version number"() { + def "versioning service throws EntityNotFoundException for non existent version number"() { when: 'Initial version' EntityDescriptor ed = new EntityDescriptor(entityID: 'ed', serviceProviderName: 'SP1', createdBy: 'anonymousUser') ed = edu.internet2.tier.shibboleth.admin.ui.repository.envers.EnversTestsSupport.doInExplicitTransaction(txMgr) { entityDescriptorRepository.save(ed) } - def edRepresentation = entityDescriptorVersionService.findSpecificVersionOfEntityDescriptor(ed.resourceId, '1000') - + then: - !edRepresentation + try { + def edRepresentation = entityDescriptorVersionService.findSpecificVersionOfEntityDescriptor(ed.resourceId, '1000') + 1 == 2 + } + catch (EntityNotFoundException expected) { + 1 == 1 + } } } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/Role.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/Role.java index 64792774d..ea3048b32 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/Role.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/model/Role.java @@ -42,7 +42,7 @@ public Role(String name, int rank) { private String name; @Column(name = "ROLE_RANK") - private int rank; + private int rank; // 0=ADMIN, additional ranks are higher //Ignore properties annotation here is to prevent stack overflow recursive error during JSON serialization @JsonIgnoreProperties("roles") diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/EntityDescriptorVersionService.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/EntityDescriptorVersionService.java index 3e2f52405..c8c67fbc8 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/EntityDescriptorVersionService.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/EntityDescriptorVersionService.java @@ -14,5 +14,5 @@ public interface EntityDescriptorVersionService { List findVersionsForEntityDescriptor(String resourceId) throws EntityNotFoundException; - EntityDescriptorRepresentation findSpecificVersionOfEntityDescriptor(String resourceId, String versionId); + EntityDescriptorRepresentation findSpecificVersionOfEntityDescriptor(String resourceId, String versionId) throws EntityNotFoundException; } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/EnversEntityDescriptorVersionService.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/EnversEntityDescriptorVersionService.java index eedf86da4..99906882b 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/EnversEntityDescriptorVersionService.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/EnversEntityDescriptorVersionService.java @@ -32,8 +32,11 @@ public List findVersionsForEntityDescriptor(String resourceId) throws E } @Override - public EntityDescriptorRepresentation findSpecificVersionOfEntityDescriptor(String resourceId, String versionId) { + public EntityDescriptorRepresentation findSpecificVersionOfEntityDescriptor(String resourceId, String versionId) throws EntityNotFoundException { Object edObject = enversVersionServiceSupport.findSpecificVersionOfPersistentEntity(resourceId, versionId, EntityDescriptor.class); - return edObject == null ? null : entityDescriptorService.createRepresentationFromDescriptor((EntityDescriptor) edObject); + if (edObject == null) { + throw new EntityNotFoundException("Unable to find specific version requested - version: " + versionId); + } + return entityDescriptorService.createRepresentationFromDescriptor((EntityDescriptor) edObject); } } From 47cd01dfac5f7ef89c6999803f2df9505d7c42c5 Mon Sep 17 00:00:00 2001 From: Ryan Mathis Date: Tue, 13 Jul 2021 12:58:53 -0700 Subject: [PATCH 29/61] Fixed group management --- .../main/resources/i18n/messages.properties | 4 + ui/src/app/admin/component/GroupForm.js | 5 +- ui/src/app/admin/container/EditGroup.js | 14 +++- ui/src/app/admin/container/GroupsList.js | 10 ++- ui/src/app/admin/container/NewGroup.js | 14 +++- ui/src/app/admin/hoc/GroupsProvider.js | 13 +++- ui/src/app/dashboard/view/SourcesTab.js | 2 +- ui/src/app/form/FormManager.js | 2 - .../app/metadata/component/MetadataHeader.js | 78 +++++++++++++++++-- ui/src/app/metadata/hoc/MetadataSelector.js | 16 +++- ui/src/app/metadata/view/MetadataOptions.js | 4 +- 11 files changed, 138 insertions(+), 24 deletions(-) diff --git a/backend/src/main/resources/i18n/messages.properties b/backend/src/main/resources/i18n/messages.properties index 9f3e5b20f..6173587f4 100644 --- a/backend/src/main/resources/i18n/messages.properties +++ b/backend/src/main/resources/i18n/messages.properties @@ -65,6 +65,7 @@ action.add-new-group=Add new group action.add-attribute=Add Attribute action.custom-entity-attributes=Custom Entity Attributes action.groups=Groups +action.source-group=Group value.enabled=Enabled value.disabled=Disabled @@ -491,6 +492,9 @@ label.provider=Metadata Provider message.delete-user-title=Delete User? message.delete-user-body=You are requesting to delete a user. If you complete this process the user will be removed. This cannot be undone. Do you wish to continue? +message.delete-group-title=Delete Group? +message.delete-group-body=You are requesting to delete a group. If you complete this process the group will be removed. This cannot be undone. Do you wish to continue? + message.delete-attribute-title=Delete Attribute? message.delete-attribute-body=You are requesting to delete a custom attribute. If you complete this process the attribute will be removed. This cannot be undone. Do you wish to continue? diff --git a/ui/src/app/admin/component/GroupForm.js b/ui/src/app/admin/component/GroupForm.js index 37e3b73dc..da8c5af8b 100644 --- a/ui/src/app/admin/component/GroupForm.js +++ b/ui/src/app/admin/component/GroupForm.js @@ -8,7 +8,7 @@ import Translate from '../../i18n/components/translate'; import { useGroupUiSchema } from '../hooks'; import { fields, widgets } from '../../form/component'; import { templates } from '../../form/component'; -import { FormContext, setFormDataAction } from '../../form/FormManager'; +import { FormContext, setFormDataAction, setFormErrorAction } from '../../form/FormManager'; function ErrorListTemplate() { return (<>); @@ -17,8 +17,9 @@ function ErrorListTemplate() { export function GroupForm ({group = {}, errors = [], loading = false, schema, onSave, onCancel}) { const { dispatch } = React.useContext(FormContext); - const onChange = ({formData}) => { + const onChange = ({formData, errors}) => { dispatch(setFormDataAction(formData)); + dispatch(setFormErrorAction(errors)); }; const uiSchema = useGroupUiSchema(); diff --git a/ui/src/app/admin/container/EditGroup.js b/ui/src/app/admin/container/EditGroup.js index 40e2a61e1..066350356 100644 --- a/ui/src/app/admin/container/EditGroup.js +++ b/ui/src/app/admin/container/EditGroup.js @@ -9,11 +9,16 @@ import { FormManager } from '../../form/FormManager'; import { GroupForm } from '../component/GroupForm'; import { GroupProvider } from '../hoc/GroupProvider'; +import { createNotificationAction, NotificationTypes, useNotificationDispatcher } from '../../notifications/hoc/Notifications'; +import { useTranslator } from '../../i18n/hooks'; export function EditGroup() { const { id } = useParams(); + const notifier = useNotificationDispatcher(); + const translator = useTranslator(); + const history = useHistory(); const { put, response, loading } = useGroups(); @@ -21,9 +26,16 @@ export function EditGroup() { const [blocking, setBlocking] = React.useState(false); async function save(metadata) { - await put(``, metadata); + let toast; + const resp = await put(``, metadata); if (response.ok) { gotoDetail({ refresh: true }); + toast = createNotificationAction(`Updated group successfully.`, NotificationTypes.SUCCESS); + } else { + toast = createNotificationAction(`${resp.errorCode} - ${translator(resp.errorMessage)}`, NotificationTypes.ERROR); + } + if (toast) { + notifier(toast); } }; diff --git a/ui/src/app/admin/container/GroupsList.js b/ui/src/app/admin/container/GroupsList.js index 07090f412..d86ff11b0 100644 --- a/ui/src/app/admin/container/GroupsList.js +++ b/ui/src/app/admin/container/GroupsList.js @@ -16,7 +16,7 @@ export function GroupsList({ groups, onDelete }) { } return ( - + {(block) =>
@@ -47,10 +47,10 @@ export function GroupsList({ groups, onDelete }) { - {(groups?.length > 0 ) ? groups.map((group, i) => + {( groups?.length > 0 ) ? groups.map((group, i) => {group.name} - {group.description} + {group.description || ''} @@ -66,7 +66,9 @@ export function GroupsList({ groups, onDelete }) { - ) : ''} + ) : + No groups defined. + }
diff --git a/ui/src/app/admin/container/NewGroup.js b/ui/src/app/admin/container/NewGroup.js index 1fabd0508..28994abd0 100644 --- a/ui/src/app/admin/container/NewGroup.js +++ b/ui/src/app/admin/container/NewGroup.js @@ -7,17 +7,29 @@ import { Schema } from '../../form/Schema'; import { FormManager } from '../../form/FormManager'; import { GroupForm } from '../component/GroupForm'; +import { createNotificationAction, NotificationTypes, useNotificationDispatcher } from '../../notifications/hoc/Notifications'; +import { useTranslator } from '../../i18n/hooks'; + export function NewGroup() { const history = useHistory(); + const notifier = useNotificationDispatcher(); + const translator = useTranslator(); const { post, response, loading } = useGroups({}); const [blocking, setBlocking] = React.useState(false); async function save(group) { - await post(``, group); + let toast; + const resp = await post(``, group); if (response.ok) { gotoDetail({ refresh: true }); + toast = createNotificationAction(`Added group successfully.`, NotificationTypes.SUCCESS); + } else { + toast = createNotificationAction(`${resp.errorCode} - ${translator(resp.errorMessage)}`, NotificationTypes.ERROR); + } + if (toast) { + notifier(toast); } }; diff --git a/ui/src/app/admin/hoc/GroupsProvider.js b/ui/src/app/admin/hoc/GroupsProvider.js index 256235b05..016bcee13 100644 --- a/ui/src/app/admin/hoc/GroupsProvider.js +++ b/ui/src/app/admin/hoc/GroupsProvider.js @@ -1,10 +1,14 @@ import React from 'react'; import { useGroups } from '../hooks'; +import { createNotificationAction, NotificationTypes, useNotificationDispatcher } from '../../notifications/hoc/Notifications'; +import { useTranslator } from '../../i18n/hooks'; export function GroupsProvider({ children, cache = 'no-cache' }) { const [groups, setGroups] = React.useState([]); + const notifier = useNotificationDispatcher(); + const translator = useTranslator(); const { get, del, response, loading } = useGroups({ cachePolicy: cache @@ -18,9 +22,16 @@ export function GroupsProvider({ children, cache = 'no-cache' }) { } async function removeGroup(id) { - await del(`/${id}`); + let toast; + const resp = await del(`/${id}`); if (response.ok) { loadGroups(); + toast = createNotificationAction(`Deleted group successfully.`, NotificationTypes.SUCCESS); + } else { + toast = createNotificationAction(`${resp.errorCode} - ${translator(resp.errorMessage)}`, NotificationTypes.ERROR); + } + if (toast) { + notifier(toast); } } diff --git a/ui/src/app/dashboard/view/SourcesTab.js b/ui/src/app/dashboard/view/SourcesTab.js index e6d464bff..dd0925876 100644 --- a/ui/src/app/dashboard/view/SourcesTab.js +++ b/ui/src/app/dashboard/view/SourcesTab.js @@ -32,7 +32,7 @@ export function SourcesTab () { React.useEffect(() => { loadSources() }, []); async function changeSourceGroup(source, group) { - const sources = await updater.put(`/${source.id}`, { + await updater.put(`/${source.id}`, { ...source, groupId: group }); diff --git a/ui/src/app/form/FormManager.js b/ui/src/app/form/FormManager.js index 17691970c..029c49c3c 100644 --- a/ui/src/app/form/FormManager.js +++ b/ui/src/app/form/FormManager.js @@ -57,8 +57,6 @@ function FormManager({ children, initial = {} }) { ...initialState, data }); - - const contextValue = React.useMemo(() => ({ state, dispatch }), [state, dispatch]); return ( diff --git a/ui/src/app/metadata/component/MetadataHeader.js b/ui/src/app/metadata/component/MetadataHeader.js index d8a2baa85..83ef7cee1 100644 --- a/ui/src/app/metadata/component/MetadataHeader.js +++ b/ui/src/app/metadata/component/MetadataHeader.js @@ -1,21 +1,82 @@ import React from 'react'; import FormattedDate from '../../core/components/FormattedDate'; +import { useIsAdmin } from '../../core/user/UserContext'; import Translate from '../../i18n/components/translate'; +import { GroupsProvider } from '../../admin/hoc/GroupsProvider'; +import { useMetadataEntity } from '../hooks/api'; +import { createNotificationAction, NotificationTypes, useNotificationDispatcher } from '../../notifications/hoc/Notifications'; +import { useTranslator } from '../../i18n/hooks'; +import { useMetadataLoader } from '../hoc/MetadataSelector'; + +export function MetadataHeader ({ showGroup, model, current = true, enabled = true, children, ...props }) { + + const isAdmin = useIsAdmin(); + const translator = useTranslator(); + + const { put, response } = useMetadataEntity('source', { + cachePolicy: 'no-cache' + }); + + const notifier = useNotificationDispatcher(); + + async function changeSourceGroup(s, group) { + let toast; + const resp = await put(`/${s.id}`, { + ...s, + groupId: group + }); + if (response.ok) { + toast = createNotificationAction(`Updated group successfully.`, NotificationTypes.SUCCESS); + reload(); + } else { + toast = createNotificationAction(`${resp.errorCode} - ${translator(resp.errorMessage)}`, NotificationTypes.ERROR); + } + if (toast) { + notifier(toast); + } + } + + const reload = useMetadataLoader(); -export function MetadataHeader ({ model, current = true, enabled = true, children, ...props }) { return (
- Saved:  - - - -
- By:  - {model.createdBy } +

+ Saved:  + + + +

+

+ By:  + {model.createdBy } +

+ {isAdmin && showGroup && + + {(groups, removeGroup, loadingGroups) => +
+ + + +
+ } + + }
{children}
@@ -29,6 +90,7 @@ export function MetadataHeader ({ model, current = true, enabled = true, childre Current

+
); diff --git a/ui/src/app/metadata/hoc/MetadataSelector.js b/ui/src/app/metadata/hoc/MetadataSelector.js index 7d7155066..acdf115be 100644 --- a/ui/src/app/metadata/hoc/MetadataSelector.js +++ b/ui/src/app/metadata/hoc/MetadataSelector.js @@ -4,6 +4,7 @@ import { useMetadataEntity } from '../hooks/api'; export const MetadataTypeContext = React.createContext(); export const MetadataObjectContext = React.createContext(); +export const MetadataLoaderContext = React.createContext(); /*eslint-disable react-hooks/exhaustive-deps*/ export function MetadataSelector({ children, ...props }) { @@ -31,10 +32,15 @@ export function MetadataSelector({ children, ...props }) { setMetadata(source); } } - React.useEffect(() => { loadMetadata(id) }, [id]); + + function reload() { + loadMetadata(id); + } + + React.useEffect(() => reload(), [id]); return ( - <> + {type && {metadata && metadata.version && @@ -42,7 +48,7 @@ export function MetadataSelector({ children, ...props }) { } } - + ); } @@ -50,4 +56,8 @@ export function useMetadataObject () { return React.useContext(MetadataObjectContext); } +export function useMetadataLoader() { + return React.useContext(MetadataLoaderContext); +} + export default MetadataSelector; \ No newline at end of file diff --git a/ui/src/app/metadata/view/MetadataOptions.js b/ui/src/app/metadata/view/MetadataOptions.js index 93011229a..38bd4a951 100644 --- a/ui/src/app/metadata/view/MetadataOptions.js +++ b/ui/src/app/metadata/view/MetadataOptions.js @@ -21,6 +21,7 @@ import { MetadataFilterTypes } from '../domain/filter'; import { useMetadataSchema } from '../hooks/schema'; import { FilterableProviders } from '../domain/provider'; + export function MetadataOptions () { const metadata = React.useContext(MetadataObjectContext); @@ -60,7 +61,8 @@ export function MetadataOptions () { + model={metadata} + showGroup={type === 'source'}> {type === 'source' && onDeleteSource &&