-
Notifications
You must be signed in to change notification settings - Fork 1
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
1125 implement permission system algorithm #1126
Closed
Closed
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
0418b31
feat: algo prototype
emsoraffa bf38ac9
feat: add annotations
emsoraffa 1b012d9
feat: added tests
emsoraffa e9730cc
Merge branch '1060-implement-permission-system-1' into 1125-implement…
emsoraffa 785c8e6
remove old file
emsoraffa 928afe6
fix: formatting, test assertion, naming
emsoraffa File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
from __future__ import annotations | ||
|
||
|
||
class PermissionDictionary: | ||
def __init__(self, dictionary: dict): | ||
self.dict = {k: set(v) for k, v in dictionary.items()} | ||
self.index_array = list(dictionary.keys()) | ||
|
||
def __getitem__(self, key: int) -> str: | ||
return ''.join(sorted(self.dict[self.index_array[key]])) | ||
|
||
def __str__(self) -> str: | ||
return str({k: ''.join(sorted(v)) for k, v in self.dict.items()}) | ||
|
||
def get(self, index: int) -> set: | ||
return self.dict[self.index_array[index]] | ||
|
||
def set(self, key: str, value: str) -> None: | ||
if key in self.dict: | ||
self.dict[key].update(value) | ||
else: | ||
raise KeyError('Key not found in permissions dictionary') | ||
|
||
def get_index_array(self) -> list: | ||
return self.index_array | ||
|
||
|
||
class Graph: | ||
def __init__(self, vertices: int, graph: list): | ||
self.V = vertices | ||
self.graph = graph | ||
|
||
def __str__(self) -> str: | ||
matrix_str = '' | ||
for row in self.graph: | ||
for value in row: | ||
matrix_str += '%7d\t' % value | ||
matrix_str += '\n' | ||
return matrix_str | ||
|
||
def compute_permissions(self, permissions: PermissionDictionary) -> PermissionDictionary: | ||
reach = [row[:] for row in self.graph] | ||
index_array = permissions.get_index_array() | ||
|
||
for k in range(self.V): | ||
for i in range(self.V): | ||
for j in range(self.V): | ||
if reach[i][j] or (reach[i][k] and reach[k][j]): | ||
reach[i][j] = 1 | ||
permissions.set(index_array[j], permissions.get(i)) | ||
# Update the graph's own matrix to reflect the transitive closure | ||
self.graph = reach | ||
return permissions |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
from __future__ import annotations | ||
|
||
import pytest | ||
|
||
from root.utils.permission_algorithm import Graph, PermissionDictionary | ||
|
||
|
||
def test_graph_compute_permissions(): | ||
# Setup graph and permissions data. | ||
graph_data = [ | ||
[1, 0, 1, 0, 0], | ||
[0, 1, 1, 0, 1], | ||
[0, 0, 1, 1, 0], | ||
[0, 0, 0, 1, 0], | ||
[0, 0, 0, 0, 1], | ||
] | ||
permissions_data = { | ||
'Ansatt': 'B', | ||
'Hest': 'A', | ||
'Mellomleder': 'C', | ||
'Sjef': 'D', | ||
'Baltazar': 'E', | ||
} | ||
|
||
# Create instances of Graph and Permissions | ||
p = PermissionDictionary(permissions_data) | ||
g = Graph(5, graph_data) | ||
|
||
# Compute permissions | ||
updated_permissions = g.compute_permissions(p) | ||
|
||
# Define expected results | ||
expected_permissions = {'Ansatt': 'B', 'Hest': 'A', 'Mellomleder': 'ABC', 'Sjef': 'ABCD', 'Baltazar': 'AE'} | ||
|
||
# Assertions to check the correctness of the permissions computation | ||
for key, value in expected_permissions.items(): | ||
assert ''.join(sorted(updated_permissions.dict[key])) == value, f'Mismatch in permissions for {key}' | ||
|
||
expected_graph = [[1, 0, 1, 1, 0], [0, 1, 1, 1, 1], [0, 0, 1, 1, 0], [0, 0, 0, 1, 0], [0, 0, 0, 0, 1]] | ||
assert g.graph == expected_graph, 'Graph transitive closure incorrect' | ||
|
||
|
||
if __name__ == '__main__': | ||
pytest.main() |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wow, hva skjer nå?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
O(n^3)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Her må jeg dessverre inn som kritisk pang
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hva er bakgrunnen til alt dette?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beklager, er ikke meningen å være kjip. Jeg virker strengere over tekst enn det som er meningen, er vanskelig å unngå. Det er sikkert en kul algoritme Emil, har ikke satt meg så dypt inn i koden ennå. Jeg frykter bare at dette er et villspor. Ved første øyeblikk virker det ikke lesbart eller intuitivt. Når matriser og grafer trekkes inn blir jeg noe usikker. Å treffe O(N^3) må jeg se nærmere på, tror ikke en løsning trenger å være mer enn O(N) og O(1).
Hvor lenge har dere jobbet med dette? Mathias startet på dette i fjor, men ser det er gjort en helomvending jeg ikke helt har fått med meg. Tror det hadde vært lurt å ta med meg i loopen her tidligere. Jeg har tilfeldigvis jobbet masse med rollesystem det siste året. Å gå helt offroad med noe som ikke er kompatibelt med Django har svært stor risiko for komplikasjoner senere, og jeg tror ikke vi klarer å tenke ut alt som kan skje. Jeg er usikker på hvordan dette skalerer på sikt. Men jeg trenger å forstå mer om hvordan dere har kommet frem til denne løsningen først. Jeg forstår ikke per nå helt hvordan dette kobles opp mot brukersystem og hvordan det skal være dynamisk og fleksibelt. Har dere lest Django sin dokumentasjon om Backends? Systemet deres kan faktisk modifiseres ganske mye etter behov. Vi bør ha svært gode grunner til å divergere i en såpass kritisk del av koden, og trestruktur-behovet trenger jeg mer innsikt i. Har dere en konkret use-case dere kan beskrive for meg? Kanskje finnes det løsninger der arv kan unngås 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Det kan også hende at jeg har misforstått noe her.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Funker denne løsningen på tabell-nivå, kolonne-nivå, rad-nivå?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Har ikke mye erfaring med Django Groups så skal ikke uttale meg om hva som er eller ikke er mulig, den avgjørelsen ble tatt før jeg begynte å se på permissions. Tror dette heller burde diskuteres på arbeidskveld enn her. Som nevnt har jeg laget en pdf som forklarer mye bedre enn jeg får til her.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Er nok lurere det ja.