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

Add create-vaults role #296

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- This CHANGELOG file
- WoT: Users will now have an ECDH as well as ECDSA key (#282)
- WoT: Users can now mutually verify their identity, hardening Hub against injection of malicious public keys (#281)
- Permission to create new vaults can now be controlled via the `create-vaults` role in Keycloak (#206)

### Changed

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ public VaultDto get(@PathParam("vaultId") UUID vaultId) {

@PUT
@Path("/{vaultId}")
@RolesAllowed("user")
@RolesAllowed("create-vaults")
@VaultRole(value = VaultAccess.Role.OWNER, onMissingVault = VaultRole.OnMissingVault.PASS)
@Consumes(MediaType.APPLICATION_JSON)
@Produces(MediaType.APPLICATION_JSON)
Expand Down Expand Up @@ -439,7 +439,7 @@ public Response createOrUpdate(@PathParam("vaultId") UUID vaultId, @Valid @NotNu

@POST
@Path("/{vaultId}/claim-ownership")
@RolesAllowed("user")
@RolesAllowed("create-vaults")
@Consumes(MediaType.APPLICATION_FORM_URLENCODED)
@Transactional
@Operation(summary = "claims ownership of a vault",
Expand Down
12 changes: 9 additions & 3 deletions backend/src/main/resources/dev-realm.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,19 @@
"description": "User",
"composite": false
},
{
"name": "create-vaults",
"description": "Can create vaults",
"composite": false
},
{
"name": "admin",
"description": "Administrator",
"composite": true,
"composites": {
"realm": [
"user"
"user",
"create-vaults"
],
"client": {
"realm-management": [
Expand Down Expand Up @@ -73,7 +79,7 @@
"email": "alice@localhost",
"enabled": true,
"credentials": [{"type": "password", "value": "asd"}],
"realmRoles": ["user"]
"realmRoles": ["user", "create-vaults"]
},
{
"username": "bob",
Expand All @@ -82,7 +88,7 @@
"email": "bob@localhost",
"enabled": true,
"credentials": [{"type": "password", "value": "asd"}],
"realmRoles": ["user"]
"realmRoles": ["user", "create-vaults"]
},
{
"username": "carol",
Expand Down
36 changes: 33 additions & 3 deletions backend/src/test/java/org/cryptomator/hub/api/VaultResourceIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -245,13 +245,26 @@ public void testUnlock() {

@Nested
@DisplayName("As vault admin user1")
@TestSecurity(user = "User Name 1", roles = {"user"})
@TestSecurity(user = "User Name 1", roles = {"create-vaults"})
@OidcSecurity(claims = {
@Claim(key = "sub", value = "user1")
})
@TestMethodOrder(MethodOrderer.OrderAnnotation.class)
public class CreateVaults {

@Test
@Order(1)
@TestSecurity(user = "User Name 1", roles = {"user"})
@DisplayName("PUT /vaults/7E57C0DE-0000-4000-8000-000100003333 returns 403 for missing role")
public void testCreteVaultWithMissingRole() {
var uuid = UUID.fromString("7E57C0DE-0000-4000-8000-000100003333");
var vaultDto = new VaultResource.VaultDto(uuid, "My Vault", "Test vault 3", false, Instant.parse("2112-12-21T21:12:21Z"), "masterkey3", 42, "NaCl", "authPubKey3", "authPrvKey3");

given().contentType(ContentType.JSON).body(vaultDto)
.when().put("/vaults/{vaultId}", "7E57C0DE-0000-4000-8000-000100003333")
.then().statusCode(403);
}

@Test
@Order(1)
@DisplayName("PUT /vaults/7E57C0DE-0000-4000-8000-000100003333 returns 201")
Expand Down Expand Up @@ -647,7 +660,7 @@ public void getMembersOfVault1b() {

@Nested
@DisplayName("When exceeding 5 seats in license")
@TestSecurity(user = "User Name 1", roles = {"user"})
@TestSecurity(user = "User Name 1", roles = {"user", "create-vaults"})
@OidcSecurity(claims = {
@Claim(key = "sub", value = "user1")
})
Expand Down Expand Up @@ -832,7 +845,7 @@ public void reset() throws SQLException {

@Nested
@DisplayName("Claim Ownership")
@TestSecurity(user = "User Name 1", roles = {"user"})
@TestSecurity(user = "User Name 1", roles = {"create-vaults"})
@OidcSecurity(claims = {
@Claim(key = "sub", value = "user1")
})
Expand Down Expand Up @@ -994,6 +1007,23 @@ public void testClaimOwnershipNoSuchVault() {
.then().statusCode(404);
}

@Test
@Order(1)
@TestSecurity(user = "User Name 1", roles = {"user"})
@DisplayName("POST /vaults/7E57C0DE-0000-4000-8000-000100009999/claim-ownership returns 403 for missing role")
public void testClaimOwnershipWithMissingRole() {
var proof = JWT.create()
.withNotBefore(Instant.now().minusSeconds(10))
.withExpiresAt(Instant.now().plusSeconds(10))
.withSubject("user1")
.withClaim("vaultId", "7E57C0DE-0000-4000-8000-000100009999".toLowerCase())
.sign(JWT_ALG);

given().param("proof", proof)
.when().post("/vaults/{vaultId}/claim-ownership", "7E57C0DE-0000-4000-8000-000100009999")
.then().statusCode(403);
}

@Test
@Order(2)
@DisplayName("POST /vaults/7E57C0DE-0000-4000-8000-000100009999/claim-ownership returns 200")
Expand Down
8 changes: 2 additions & 6 deletions frontend/src/common/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,8 @@ class Auth {
return this.keycloak.token;
}

public isAdmin(): boolean {
return this.keycloak.tokenParsed?.realm_access?.roles.includes('admin') ?? false;
}

public isUser(): boolean {
return this.keycloak.tokenParsed?.realm_access?.roles.includes('user') ?? false;
public hasRole(role: string): boolean {
return this.keycloak.tokenParsed?.realm_access?.roles.includes(role) ?? false;
}
}

Expand Down
18 changes: 18 additions & 0 deletions frontend/src/components/Forbidden.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<template>
<div class="bg-white min-h-full px-4 py-16 sm:px-6 sm:py-24 md:grid md:place-items-center lg:px-8">
<div class="max-w-max mx-auto">
<main class="sm:flex">
<p class="text-4xl font-extrabold text-primary sm:text-5xl">403</p>
<div class="sm:ml-6">
<div class="sm:border-l sm:border-gray-200 sm:pl-6">
<h1 class="text-4xl font-extrabold text-gray-900 tracking-tight sm:text-5xl">Forbidden</h1>
<p class="mt-1 text-base text-gray-500">You don't have the permission to access this page.</p>
</div>
<div class="mt-10 flex space-x-3 sm:border-l sm:border-transparent sm:pl-6">
<router-link to="/app/vaults" class="inline-flex items-center px-4 py-2 border border-transparent text-sm font-medium rounded-md shadow-sm text-white bg-primary focus:outline-none focus:ring-2 focus:ring-offset-2 focus:ring-primary"> Go back home </router-link>
</div>
</div>
</main>
</div>
</div>
</template>
2 changes: 1 addition & 1 deletion frontend/src/components/NavigationBar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ const props = defineProps<{
}>();

onMounted(async () => {
if ((await auth).isAdmin()) {
if ((await auth).hasRole('admin')) {
profileDropdown.value = [profileDropdownSections.infoSection, profileDropdownSections.adminSection, profileDropdownSections.hubSection];
} else {
profileDropdown.value = [profileDropdownSections.infoSection, profileDropdownSections.hubSection];
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/components/VaultDetails.vue
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ onMounted(fetchData);
async function fetchData() {
onFetchError.value = null;
try {
isAdmin.value = (await auth).isAdmin();
isAdmin.value = (await auth).hasRole('admin');
vault.value = await backend.vaults.get(props.vaultId);
me.value = await userdata.me;
license.value = await backend.license.getUserInfo();
Expand Down
8 changes: 5 additions & 3 deletions frontend/src/components/VaultList.vue
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@

<Menu as="div" class="relative inline-block text-left">
<div>
<MenuButton :disabled="isLicenseViolated" class="inline-flex items-center px-4 py-2 border border-transparent rounded-md shadow-sm text-sm font-medium text-white bg-primary hover:bg-primary-d1 focus:outline-none focus:ring-2 focus:ring-offset-2 focus:ring-primary" :class="{ 'cursor-not-allowed opacity-50': isLicenseViolated }">
<MenuButton :disabled="isLicenseViolated || !canCreateVaults" class="inline-flex items-center px-4 py-2 border border-transparent rounded-md shadow-sm text-sm font-medium text-white bg-primary hover:bg-primary-d1 focus:outline-none focus:ring-2 focus:ring-offset-2 focus:ring-primary disabled:cursor-not-allowed disabled:opacity-50">
{{ t('vaultList.addVault') }}
<ChevronDownIcon class="-mr-1 ml-2 h-5 w-5" aria-hidden="true" />
</MenuButton>
Expand Down Expand Up @@ -147,7 +147,8 @@ const roleOfSelectedVault = computed<VaultRole | 'NONE'>(() => {
}
});

const isAdmin = ref<boolean>();
const isAdmin = ref<boolean>(false);
const canCreateVaults = ref<boolean>(false);
const licenseStatus = ref<LicenseUserInfoDto>();
const isLicenseViolated = computed(() => {
if (licenseStatus.value) {
Expand Down Expand Up @@ -177,7 +178,8 @@ onMounted(fetchData);
async function fetchData() {
onFetchError.value = null;
try {
isAdmin.value = (await auth).isAdmin();
isAdmin.value = (await auth).hasRole('admin');
canCreateVaults.value = (await auth).hasRole('create-vaults');
overheadhunter marked this conversation as resolved.
Show resolved Hide resolved

if (isAdmin.value) {
filterOptions.value['allVaults'] = t('vaultList.filter.entry.allVaults');
Expand Down
31 changes: 24 additions & 7 deletions frontend/src/router/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { createRouter, createWebHistory, RouteLocationRaw, RouteRecordRaw } from 'vue-router';
import { createRouter, createWebHistory, NavigationGuardWithThis, RouteLocationRaw, RouteRecordRaw } from 'vue-router';
import authPromise from '../common/auth';
import backend from '../common/backend';
import { baseURL } from '../common/config';
Expand All @@ -14,6 +14,19 @@ import UnlockSuccess from '../components/UnlockSuccess.vue';
import UserProfile from '../components/UserProfile.vue';
import VaultDetails from '../components/VaultDetails.vue';
import VaultList from '../components/VaultList.vue';
import Forbidden from '../components/Forbidden.vue';

function checkRole(role: string): NavigationGuardWithThis<undefined> {
return async (to, _) => {
const auth = await authPromise;
if (auth.hasRole(role)) {
return true;
} else {
console.warn(`Access denied: User requires role ${role} to access ${to.fullPath}`);
return { path: '/app/forbidden', replace: true };
}
};
}
overheadhunter marked this conversation as resolved.
Show resolved Hide resolved

const routes: RouteRecordRaw[] = [
{
Expand Down Expand Up @@ -52,12 +65,14 @@ const routes: RouteRecordRaw[] = [
{
path: 'vaults/create',
component: CreateVault,
props: () => ({ recover: false })
props: () => ({ recover: false }),
beforeEnter: checkRole('create-vaults'),
},
{
path: 'vaults/recover',
component: CreateVault,
props: () => ({ recover: true })
props: () => ({ recover: true }),
beforeEnter: checkRole('create-vaults'),
},
{
path: 'vaults/:id',
Expand All @@ -70,10 +85,7 @@ const routes: RouteRecordRaw[] = [
},
{
path: 'admin',
beforeEnter: async () => {
const auth = await authPromise;
return auth.isAdmin(); //TODO: reroute to NotFound Screen/ AccessDeniedScreen?
},
beforeEnter: checkRole('admin'),
children: [
{
path: '',
Expand Down Expand Up @@ -114,6 +126,11 @@ const routes: RouteRecordRaw[] = [
component: NotFound,
meta: { skipAuth: true, skipSetup: true }
},
{
path: '/app/forbidden',
component: Forbidden,
meta: { skipAuth: true, skipSetup: true }
},
];

const router = createRouter({
Expand Down