Skip to content

BA Meyer final commit #19

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 76 commits into from
May 27, 2022
Merged

BA Meyer final commit #19

merged 76 commits into from
May 27, 2022

Conversation

lqwert
Copy link
Contributor

@lqwert lqwert commented Oct 19, 2020 •
edited by ghareeb-falazi
Loading

Finale PR of my BA code of the Pattern Atlas API

Related to: PatternAtlas/pattern-atlas-ui#68
and
PatternAtlas/pattern-atlas-docker#9

@lqwert lqwert requested a review from manuwei October 19, 2020 15:36
@manuwei manuwei mentioned this pull request Jun 22, 2021
Open
@ghost
Copy link

ghost commented Jul 15, 2021 •
edited by ghost
Loading

Congratulations 🎉. DeepCode analyzed your code in 3.795 seconds and we found no issues. Enjoy a moment of no bugs ☀️.

👉 View analysis in DeepCode’s Dashboard | Configure the bot

👉 The DeepCode service and API will be deprecated in August, 2021. Here is the information how to migrate. Thank you for using DeepCode 🙏 ❤️ !

If you are using our plugins, you might be interested in their successors: Snyk's JetBrains plugin and Snyk's VS Code plugin.

@TYueksel TYueksel mentioned this pull request Sep 21, 2021
Open
mhinkie and others added 24 commits April 4, 2022 15:22
@mhinkie
moved privilege checks
9d69d52
@mhinkie
removed swagger ui
77a5ea6
@mhinkie
changed resource server config for removed swagger
8e2541e
@mhinkie
added role management for authors in candidateservice
c4b2483
@mhinkie
fixed author deleting for issue
e7db347
@mhinkie
removed in-code checks for privileges
430107c
@mhinkie
fixed candidate service privilege check
7a297a8
@mhinkie
added function for changing platform wide roles for users
8ff69f4
@PhilWun
add endpoint to get all default privileges
7f25d80
@PhilWun
improve endpoint naming
294aafe
@PhilWun
add endpoint to update all resource specific roles
075e9b3 
this is used to add/remove a privilege to/from an author role for all resources
@PhilWun
change http method to match the REST guidelines
224bfa9
@PhilWun
add missing permission checks
829182b
@PhilWun
add guest role to specify what a user can do who is not logged in
9b01014
@PhilWun
disable design model endpoints, because they are currently not used a…
1acd581 
…nd don't check for privileges
@PhilWun
disable discussions endpoints, because they are currently not used an…
bfda094 
…d don't check for privileges
@PhilWun
disable image endpoints, because they are currently not used and don'…
9474446 
…t check for privileges
@PhilWun
fix error when deleting pattern from pattern language
6969599
@mhinkie
added initial user creation for users created in keycloak
55a5b4d
@PhilWun
add missing permission checks, fix some existing permission checks, f…
0d295d3 
…ix some bugs that caused exceptions
@mhinkie
added logic for creating admin user on first login
75cc597
@mhinkie
changed realm
86e81ee
@mhinkie
Merge branch 'ba-meyer-master' of https://github.com/PatternAtlas/pat…
8cbbef8 
…tern-atlas-api into ba-meyer-master
@mhinkie
removed old auth logic
0d7beea
lharzenetter
lharzenetter requested changes May 25, 2022
View reviewed changes
Copy link
Member

@lharzenetter lharzenetter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general looks really good. However, I would suggest some improvements and that you run the autoformatter once again on all files.

src/main/java/io/github/patternatlas/api/entities/user/role/PrivilegeConstant.java

import org.springframework.stereotype.Component;

@Component("PC")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we use a more speaking name?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reasoning was to have a short name for it since it is used in every PreAuthorize check as for example
@PostFilter("hasResourcePermission(filterObject.id, @PC.ISSUE_READ)") and those annotations get large pretty fast and a longer name might make the actual privilege name (i.e. ISSUE_READ) harder to grasp quickly. Should I still change it?

@mhinkie mhinkie dismissed PhilWun’s stale review May 25, 2022 13:28

old review - dockerfile has changed in the meantime

@mhinkie mhinkie merged commit cb13b7d into master May 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mar-be mar-be mar-be left review comments

@mhinkie mhinkie mhinkie left review comments

@TYueksel TYueksel TYueksel left review comments

@lharzenetter lharzenetter lharzenetter requested changes

@PhilWun PhilWun PhilWun left review comments

@ghareeb-falazi ghareeb-falazi Awaiting requested review from ghareeb-falazi

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@lqwert @TYueksel @PhilWun @lharzenetter @mar-be @mhinkie