Conversation
|
|
||
| INSERT INTO application.acl (role, action, resource) | ||
| VALUES | ||
| ('trn:temboard:core:group:mass/dba', '*', 'trn:temboard:core:instance:mass'), |
There was a problem hiding this comment.
| ('trn:temboard:core:group:mass/dba', '*', 'trn:temboard:core:instance:mass'), | |
| ('trn:temboard:core:group:mass/dba', '*', 'trn:temboard:core:instance:mass/*'), |
| return [TRN("*", "*", "*")] | ||
|
|
||
|
|
||
| def configure_authorization(role=None, action=None, resource=None): |
There was a problem hiding this comment.
Où est-ce utilisé ?
Je ne vois pas l'intérêt. Autant passer directement en paramètre à check_acl().
There was a problem hiding this comment.
Comme check() est ajouté au before_request de flask, il ne me semble pas possible de lui passer des param, j'ai donc créé un décorateur pour impacter le test d'authorization sur une route en particulier, ce n'est aps utilisé pour le moment. Je peux le supprimer mais en fait je peux également supprimer les parametres de check(). Pour le moment on utilise toujours anonymous ou g.current_user comme role et g.instance ou * pour la resource.
dde5737 to
dbf480c
Compare
f1f0cd5 to
02d3e4a
Compare
Also remove admin_required and anonymous_allowed decorator
| @@ -0,0 +1,40 @@ | |||
| class TRN: | |||
There was a problem hiding this comment.
It would be nice to add a short docstring explain what TRN is, what it looks like.
| @staticmethod | ||
| def parse(trn): | ||
| elems = str.split(trn, ":") | ||
| if len(elems) < 5: | ||
| raise Exception("Malformed TRN") | ||
| return TRN(elems[2], elems[3], elems[4]) |
There was a problem hiding this comment.
I would use a class method here:
@classmethod
def parse(cls, trn):
elems = str.split(trn, ":")
if len(elems) < 5:
raise Exception("Malformed TRN")
return cls(elems[2], elems[3], elems[4])You avoid repeating the name of the class. It prevents errors if the class is renamed. And it works better with inheritance.
|
|
||
| def test_trn_expand(): | ||
| trn = TRN.parse("trn:temboard:core:user:alice") | ||
| parents = TRN.expand(trn) |
There was a problem hiding this comment.
Here you use expand just like if it was a classmethod. In the function self isn't really what it is supposed to be.
The following would be more straightforward:
parents = trn.expand()
|
|
||
| def expand(self): | ||
| trns = ["*"] | ||
| trn = TRN(self.scope, self.type, self.name) |
There was a problem hiding this comment.
self is already a TRN. You shouldn't have to recreate one.
| parent.scope = "*" | ||
| return parent | ||
|
|
||
| def expand(self): |
There was a problem hiding this comment.
What about having a property called parents instead?
| @@ -0,0 +1,56 @@ | |||
| import pytest | |||
| from temboardui.acl import TRN, expand_actions | |||
There was a problem hiding this comment.
expand_actions doesn't exist in this commit. It seems to be added in a future commit.
| assert str(parents[0]) == "*" | ||
| assert str(parents[1]) == "trn:temboard:core:user:alice" | ||
| assert str(parents[2]) == "trn:temboard:core:user:*" | ||
| assert str(parents[3]) == "trn:temboard:core:*:*" | ||
| assert str(parents[4]) == "trn:temboard:*:*:*" |
There was a problem hiding this comment.
I feels like the order of parents is reversed, starting from index 1.
This would look more natural:
assert str(parents[0]) == "*"
assert str(parents[4]) == "trn:temboard:*:*:*"
assert str(parents[3]) == "trn:temboard:core:*:*"
assert str(parents[2]) == "trn:temboard:core:user:*"
assert str(parents[1]) == "trn:temboard:core:user:alice"| groups=[g.name for g in self.groups if g.environment], | ||
| environments=[g.environment.name for g in self.groups if g.environment], |
There was a problem hiding this comment.
Is it related to current commit?
| def trn(self): | ||
| return TRN("core", "apikey", str(self.id)) | ||
|
|
||
| def role_trns(self): | ||
| return self.trn().expand() | ||
|
|
||
| def resource_trns(self): | ||
| return self.trn().expand() |
There was a problem hiding this comment.
We could possibly use @property.
3 participants
No description provided.