`Development`: Unify admin endpoints

[julian-christl]

Checklist

General

• [X] I tested all changes and their related features with all corresponding user types on a test server.
• [X] I chose a title conforming to the naming conventions for pull requests.

Server

• [X] I followed the coding and design guidelines.
• [X] I added pre-authorization annotations according to the guidelines and checked the course groups for all new REST Calls (security).

Client

• [X] I followed the coding and design guidelines and ensured that the layout is responsive.

Motivation and Context

The idea of these changes is to prepare the endpoints for automated authorization tests. Additionally, it allows for autocompletion.

Description

I moved all endpoints that are only accessible to Admins from api/... to api/admin/..., wrapped @PreAuthorize("hasRole('ADMIN')") in @EnforceAdmin and replaced all occurrences. I also adapted the documentation and the pull request template.

Steps for Testing

Look at the code and consider checking all affected endpoints. If there is no difference between endpoints in regards to the changes, testing them might be not necessary.

Steps for Code Review

I made sure that all occurrences were replaced. Please double-check that I didn't accidentally replace too much and changed the permissions. Also, ensure that I adapted all connections and didn't damage the interfaces. If you have suggestions on how to improve the separation, let me know.

Review Progress

Code Review

• [x] Review 1
• [x] Review 2

Manual Tests

• [x] Test 1
• [x] Test 2

[julian-christl]

• You should be able to put @EnforceAdmin over the class declaration in the Admin Resources, since all endpoints in there should only have admin privileges

This is possible, yes. I actually decided against that now and adapted the code. The intention is that a developer can see the required role without the need to scroll up. This also prevents overrides of the annotation.

[github-actions[bot]]

There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions.

[julian-christl]

Seams like some e2e tests are failing

I don't think this is an issue with my code. If you look on Bamboo, the last build that ran through was the merge commit. I got two commits after that:

1. Only touches two tests
2. Touches documentation, automatic renaming of two methods and only one change in the system notifications service that first should not change the functionality and second should not affect the tests that are failing now.

Considering that a rerun of this (based on the same develop commit) yields the same result, I would say this is a general issue right now.

[JohannesStoehr]

Seams like some e2e tests are failing

I don't think this is an issue with my code. If you look on Bamboo, the last build that ran through was the merge commit. I got two commits after that:

1. Only touches two tests
2. Touches documentation, automatic renaming of two methods and only one change in the system notifications service that first should not change the functionality and second should not affect the tests that are failing now.

Considering that a rerun of this (based on the same develop commit) yields the same result, I would say this is a general issue right now.

They are consistently failing in your PR, even if I rerun the tests. But they do pass on develop, so you might want to take a look at them...

[julian-christl]

Seams like some e2e tests are failing

I don't think this is an issue with my code. If you look on Bamboo, the last build that ran through was the merge commit. I got two commits after that:

1. Only touches two tests
2. Touches documentation, automatic renaming of two methods and only one change in the system notifications service that first should not change the functionality and second should not affect the tests that are failing now.

Considering that a rerun of this (based on the same develop commit) yields the same result, I would say this is a general issue right now.

They are consistently failing in your PR, even if I rerun the tests. But they do pass on develop, so you might want to take a look at them...

They do not pass on develop. The latest one (your PR) is also failing. However, it passed before the merge. See the discussion on Slack in #artemisdev we currently have. It looks like the Bamboo prelive is at fault.

[julian-christl]

Got a bit premature with the first fix but now it's working. Found here: https://github.com/ls1intum/Artemis/pull/5638#pullrequestreview-1117750106

Tested on TS1

So the only change is commit 513a838

[github-actions[bot]]

There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions.

[github-actions[bot]]

There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions.

[github-actions[bot]]

There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions.

[julian-christl]

I cherry-picked some commits, and others I reimplemented. So some new code reviews to check for consistency would make sense

[codecov-commenter]

Codecov Report

Base: 75.52% // Head: 75.55% // Increases project coverage by +0.02% :tada:

Coverage data is based on head (3e3caf6) compared to base (006df9c). Patch coverage: 78.78% of modified lines in pull request are covered.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #5574      +/-   ##
===========================================
+ Coverage    75.52%   75.55%   +0.02%     
===========================================
  Files         1105     1108       +3     
  Lines        43710    43745      +35     
  Branches      8261     8268       +7     
===========================================
+ Hits         33012    33051      +39     
  Misses        5579     5579              
+ Partials      5119     5115       -4     

Flag	Coverage Δ
client	75.55% <78.78%> (+0.02%)	:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...app/app/course/manage/course-management.service.ts	79.90% <ø> (+2.17%)	:arrow_up:
...pp/shared/feature-toggle/feature-toggle.service.ts	61.76% <0.00%> (ø)
...system-notification/system-notification.service.ts	46.42% <ø> (+3.57%)	:arrow_up:
...rc/main/webapp/app/core/user/admin-user.service.ts	44.44% <44.44%> (ø)
...gement/system-notification-management.component.ts	79.01% <66.66%> (+0.53%)	:arrow_up:
...-notification/admin-system-notification.service.ts	66.66% <66.66%> (ø)
.../app/shared/statistics-graph/statistics.service.ts	45.16% <75.00%> (+3.78%)	:arrow_up:
...pp/course/manage/detail/course-detail.component.ts	88.73% <83.33%> (+0.32%)	:arrow_up:
...n/webapp/app/course/manage/course-admin.service.ts	87.50% <87.50%> (ø)
src/main/webapp/app/admin/audits/audits.service.ts	100.00% <100.00%> (ø)
... and 10 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[julian-christl]

In #5723, we fundamentally change how authentication works. Therefore, we won't have a dedicated file authentication anymore, so I reverted the file endpoint commit.