Skip to content
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.

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

fix(permissions): reject malformed doap and ap create/update request (DSP-1328) #1890

Merged
merged 6 commits into from Jul 20, 2021

Conversation

SepidehAlassi
Copy link
Contributor

@SepidehAlassi SepidehAlassi commented Jul 14, 2021

resolves DSP-1328

  • Improve documentation about permissions
  • Reject malformed update hasPermissions requests for both AP and DOAP
  • Reject create AP and DOAP requests when hasPermissions was malformed.
  • add tests

@SepidehAlassi SepidehAlassi self-assigned this Jul 14, 2021
@SepidehAlassi SepidehAlassi added API/Admin enhancement improve existing code or new feature bug something isn't working labels Jul 14, 2021
@SepidehAlassi SepidehAlassi marked this pull request as ready for review July 15, 2021 08:52
Copy link
Collaborator

@subotic subotic left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

`hasPermissions` contains permission types that must be granted. See [the complete description of administrative
permission types](../../05-internals/design/api-admin/administration.md#administrative-permissions).
In summary, each permission should contain followings:
- `name` : indicates the type of the type of the permission that can be one of the followings:
Copy link
Collaborator

Choose a reason for hiding this comment

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

probably "the type of the type of permission" -> "the type of permission"

- `M` modify permission
- `D`: delete permission
- `CR`: change rights permission (most privileged)
- `permissionCode`: The code assigned to a permission indicating its hierarchical lever. These codes are as below:
Copy link
Collaborator

Choose a reason for hiding this comment

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

"lever" -> "level"?

@@ -88,4 +105,82 @@ object PermissionsMessagesUtilADM {

CacheUtil.remove(PermissionsCacheName, key)
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing docs


/**
* For default object access permission, we need to make sure that the value given for the projectCode matches the value of name parameter.
* This method, validates the content of hasPermissions collection by verifying that both projectCode and name indicate the same type of permission.
Copy link
Collaborator

Choose a reason for hiding this comment

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

what is meant by projectCode?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hehe that was a permissioncode, sorry for the typo.

@SepidehAlassi SepidehAlassi merged commit 3e3a3ce into main Jul 20, 2021
@SepidehAlassi SepidehAlassi deleted the wip/DSP-1328-RejectMalformedDOAPUpdateRequest branch July 20, 2021 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API/Admin bug something isn't working enhancement improve existing code or new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants