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

Feature/user permissions #142

Merged
merged 8 commits into from
Dec 9, 2024
Merged
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
9 changes: 5 additions & 4 deletions backend/conftest.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
from allauth.account.models import EmailAddress
import pytest
from graphene.test import Client
from graphene.test import Client as GrapheneClient
from typing import Generator
from django.test import Client as APIClient

from case_study.models import CaseStudy
from letter.models import LetterDescription
Expand All @@ -12,7 +14,6 @@
from space.models import SpaceDescription
from graphql_app.schema import schema


@pytest.fixture()
def user_data():
return {
Expand Down Expand Up @@ -40,7 +41,7 @@ def user(db, user_data):


@pytest.fixture
def user_client(client, user):
def user_client(client, user) -> Generator[APIClient, None, None]:
client.force_login(user)
yield client
client.logout()
Expand Down Expand Up @@ -145,5 +146,5 @@ def case_study(db):

@pytest.fixture()
def graphql_client():
client = Client(schema)
client = GrapheneClient(schema)
return client
4 changes: 2 additions & 2 deletions backend/core/tests/test_contributors.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ def setUpTestData(cls) -> None:
cls.episode = Episode.objects.create(
name="Death Star destroyed", source_id=source.pk
)
cls.user_1 = User.objects.create(username="Yoda")
cls.user_2 = User.objects.create(username="Obi-Wan")
cls.user_1 = User.objects.create(username="Yoda", is_contributor=True)
cls.user_2 = User.objects.create(username="Obi-Wan", is_contributor=True)

def test_single_contributor_one_contribution(self):
self.client.force_login(self.user_1)
Expand Down
30 changes: 15 additions & 15 deletions backend/graphql_app/middleware.py
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
import json
from django.conf import settings
from django.contrib.auth.models import AnonymousUser
from user.models import User
from typing import Union
from graphene import ResolveInfo


class GraphQLAuthMiddleware:
def resolve(self, next, root, info, **kwargs):
request = info.context
def has_mutation_permission(user: Union[AnonymousUser, User]) -> bool:
return user.is_superuser or getattr(user, "is_contributor", False)


# Allow introspection queries to pass through in development mode.
if settings.DEBUG and self.is_introspection_query(request):
return next(root, info, **kwargs)
def is_mutation(info: ResolveInfo) -> bool:
return info.parent_type.name == "Mutation"

if info.context.user.is_authenticated:
return next(root, info, **kwargs)

raise Exception("User is not authenticated")
class GraphQLAuthMiddleware:

def resolve(self, next, root, info: ResolveInfo, **kwargs):
if is_mutation(info) and not has_mutation_permission(info.context.user):
raise Exception("User is not authorised to make mutations")

def is_introspection_query(self, request):
data = json.loads(request.body)
query = data.get("query") # type: str
return query.startswith("query IntrospectionQuery")
return next(root, info, **kwargs)
58 changes: 0 additions & 58 deletions backend/graphql_app/tests.py

This file was deleted.

63 changes: 63 additions & 0 deletions backend/graphql_app/tests/test_middleware.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
from rest_framework.status import is_success
import json

QUERY_EXAMPLE = """
query TestQuery {
sources {
id
}
}
"""

MUTATION_EXAMPLE = """
mutation TestMutation {
createAgent(agentData: {name: "test", source: "1"}) {
ok
}
}
"""


def test_middleware_passes_queries(client, db):
response = client.post(
"/api/graphql",
{
"operationName": "TestQuery",
"query": QUERY_EXAMPLE,
},
)

assert is_success(response.status_code)
data = json.loads(response.content)
assert not "errors" in data


def test_middleware_blocks_mutation_from_unauthorised_user(user_client, source):
response = user_client.post(
"/api/graphql",
{
"operationName": "TestMutation",
"query": MUTATION_EXAMPLE,
},
)

assert is_success(response.status_code)
data = json.loads(response.content)
assert data["errors"][0]["message"] == "User is not authorised to make mutations"


def test_middleware_passes_mutation_from_authorised_user(user, user_client, source):
user.is_contributor = True
user.save()

response = user_client.post(
"/api/graphql",
{
"operationName": "TestMutation",
"query": MUTATION_EXAMPLE,
},
)

assert is_success(response.status_code)
data = json.loads(response.content)
assert not "errors" in data
3 changes: 2 additions & 1 deletion backend/user/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,5 @@

@admin.register(models.User)
class UserAdmin(auth_admin.UserAdmin):
pass
fieldsets = auth_admin.UserAdmin.fieldsets
fieldsets[2][1]["fields"] = list(fieldsets[2][1]["fields"]) + ["is_contributor"]
18 changes: 18 additions & 0 deletions backend/user/migrations/0004_user_is_contributor.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Generated by Django 4.2.7 on 2024-10-01 13:24

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('user', '0003_sitedomain'),
]

operations = [
migrations.AddField(
model_name='user',
name='is_contributor',
field=models.BooleanField(default=False, help_text='Whether this user is a contributor on the project; this enables them to enter or edit research data.'),
),
]
6 changes: 6 additions & 0 deletions backend/user/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,9 @@ class User(django_auth_models.AbstractUser):

class Meta:
db_table = "auth_user"

is_contributor = models.BooleanField(
default=False,
help_text="Whether this user is a contributor on the project; this enables them "
"to enter or edit research data.",
)
3 changes: 2 additions & 1 deletion backend/user/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,6 @@ class Meta(UserDetailsSerializer.Meta):
"first_name",
"last_name",
"is_staff",
"is_contributor",
)
read_only_fields = ["is_staff", "id", "email"]
read_only_fields = ["is_staff", "id", "email", "is_contributor"]
6 changes: 4 additions & 2 deletions backend/user/tests/test_user_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ def test_user_details(user_client, user_data):
"first_name": user_data["first_name"],
"last_name": user_data["last_name"],
"is_staff": False,
"is_contributor": False,
}


Expand All @@ -28,10 +29,11 @@ def test_user_updates(user_client, user_data):
assert response.status_code == 200
assert details()["username"] == "NewName"

# is_staff is readonly, so nothing should happen
# is_staff and is_contributor are readonly, so nothing should happen
response = user_client.patch(
route,
{"is_staff": True},
{"is_staff": True, "is_contributor": True},
content_type="application/json",
)
assert not details()["is_staff"]
assert not details()["is_contributor"]
2 changes: 1 addition & 1 deletion frontend/src/app/app.module.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { NgModule } from '@angular/core';

import { AppComponent } from './app.component';
import { AppRoutingModule } from './app-routing.module';
import { AppRoutingModule } from './routing/app-routing.module';

import { HomeComponent } from './home/home.component';
import { SharedModule } from './shared/shared.module';
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/app/core/menu/menu.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
Home
</a>
</li>
<li *ngIf="isAuthenticated$ | async" class="nav-item">
<li *ngIf="isContributor$ | async" class="nav-item">
<a [routerLink]="['/data-entry']" routerLinkActive="active" class="nav-link">
Data entry
</a>
Expand Down
5 changes: 4 additions & 1 deletion frontend/src/app/core/menu/menu.component.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Component } from '@angular/core';
import { AuthService } from '@services/auth.service';
import { map } from 'rxjs';

@Component({
selector: 'lc-menu',
Expand All @@ -8,7 +9,9 @@ import { AuthService } from '@services/auth.service';
})
export class MenuComponent {
public burgerActive = false;
public isAuthenticated$ = this.authService.isAuthenticated$;
public isContributor$ = this.authService.currentUser$.pipe(
map(user => user?.isContributor || false)
);

constructor(private authService: AuthService) {}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ const fakeUserResponse: UserResponse = {
email: "[email protected]",
first_name: "Frodo",
last_name: "Baggins",
is_staff: false
is_staff: false,
is_contributor: true,
}

const fakeAdminResponse: UserResponse = {
Expand All @@ -21,7 +22,8 @@ const fakeAdminResponse: UserResponse = {
email: "[email protected]",
first_name: "Gandalf",
last_name: "The Grey",
is_staff: true
is_staff: true,
is_contributor: false,
}


Expand Down
30 changes: 30 additions & 0 deletions frontend/src/app/routing/contributor.guard.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { inject } from "@angular/core";
import { CanActivateFn, Router } from "@angular/router";
import { AuthService } from "@services/auth.service";
import { ToastService } from "@services/toast.service";
import { filter, map } from "rxjs";


export const ContributorGuard: CanActivateFn = () => {
const authService = inject(AuthService);
const toastService = inject(ToastService);
const router = inject(Router);

return authService.currentUser$.pipe(
filter((user) => user !== undefined),
map((user) => {
if (user?.isContributor) {
return true;
} else {
let body = 'You do not have permission to view this page.';
if (!user) { body += ' Do you need to sign in?'; }
toastService.show({
type: 'danger',
header: 'Not authorised',
body,
});
return router.createUrlTree(['/']);
}
}),
);
};
18 changes: 18 additions & 0 deletions frontend/src/app/routing/contributor.guards.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { TestBed } from "@angular/core/testing";
import { CanActivateFn } from "@angular/router";
import { ContributorGuard } from "./contributor.guard";



describe("LoggedOnGuard", () => {
const executeGuard: CanActivateFn = (...guardParameters) =>
TestBed.runInInjectionContext(() => ContributorGuard(...guardParameters));

beforeEach(() => {
TestBed.configureTestingModule({});
});

it("should be created", () => {
expect(executeGuard).toBeTruthy();
});
});
Loading