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

Lock down bucket management to proper permissions #44

Merged
merged 3 commits into from
Mar 9, 2023
Merged

Conversation

kamorel
Copy link
Contributor

@kamorel kamorel commented Mar 7, 2023

Description

Ensures user has proper rights and permissions to action certain bucket management functionality.

Types of changes

Bug fix (non-breaking change which fixes an issue)

Checklist

  • I have read the CONTRIBUTING doc
  • I have checked that unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

@github-actions
Copy link

github-actions bot commented Mar 7, 2023

Coverage Report (Application)

Totals Coverage
Statements: 75% ( 51 / 68 )
Methods: 62.5% ( 5 / 8 )
Lines: 82.61% ( 38 / 46 )
Branches: 57.14% ( 8 / 14 )

@github-actions
Copy link

github-actions bot commented Mar 7, 2023

Coverage Report (Frontend)

Totals Coverage
Statements: 5.42% ( 42 / 775 )
Methods: 0% ( 0 / 180 )
Lines: 8.39% ( 40 / 477 )
Branches: 1.69% ( 2 / 118 )

@kamorel kamorel marked this pull request as draft March 7, 2023 21:15
@kamorel kamorel marked this pull request as ready for review March 8, 2023 00:38
Copy link
Member

@jujaga jujaga left a comment

Choose a reason for hiding this comment

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

Logically makes sense. We should make a general consistency pass on action naming conventions though to avoid naming things as get* when they aren't computed types.

frontend/src/components/bucket/BucketTable.vue Outdated Show resolved Hide resolved
frontend/src/store/permissionStore.ts Outdated Show resolved Hide resolved
frontend/src/types/options/SearchObjectsOptions.ts Outdated Show resolved Hide resolved
@kamorel kamorel requested a review from jujaga March 8, 2023 20:33
@@ -53,6 +53,7 @@ onMounted(async () => {
</div>
<div>
<Button
v-if="usePermissionStore().isUserElevatedRights()"
Copy link
Member

Choose a reason for hiding this comment

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

Should we be declaring and destructuring this in advance on setup function instead? It might end up helping this specific .js bundle be smaller in size if we only imported that one function instead of the entire permission store's methods.

<script setup lang="ts">
import { useAuthStore, useBucketStore, usePermissionStore } from '@/store';
const { isUserElevatedRights } = usePermissionStore();
</script>

<Button v-if="isUserElevatedRights()" >

Granted, this does contrast with our previous conversations about store readability (which store am I invoking from?)

frontend/src/components/bucket/BucketTable.vue Outdated Show resolved Hide resolved
@TimCsaky TimCsaky merged commit 35f0357 into master Mar 9, 2023
@jujaga jujaga deleted the bugfix/bceid branch March 10, 2023 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants