potion icon indicating copy to clipboard operation
potion copied to clipboard

Support `ToMany` in `PrincipalsMixin`

Open odcinek opened this issue 8 years ago • 6 comments

PrincipalsMixin seems to support ToOne fields, but ToMany is left unimplemented . I haven't found a good way to express it in flask-principal terms, without creating a Need instance for every relation member.

@lyschoening How would you approach this? Happy to take a shot at implementing this.

odcinek avatar Aug 12 '16 02:08 odcinek

Ah, I left this out when I implemented this because I could not think of any immediate use case. Frankly, I still cannot think of any. Permissions are usually set up in a tree structure. Do you have an example scenario?

Regarding the implementation: If we say that "any" of the items in the relationship needs to fulfil the need, then it might be enough to replace isinstance(field, ToOne) with isinstance(field, (ToOne, ToMany)). When the query is built, PrincipalsMixin calls SQLAlchemyManager._expression_for_join(), which will automatically determine the correct join to use.

It is possible that I am overlooking something. It would definitely need some tests. Since I unwisely wrote all the tests against a single database model it might also be necessary to add a new TestCase.

lyschoening avatar Aug 12 '16 08:08 lyschoening

Hi. One of the examples might be a user who is a member of a group. Case scenario: Restrict updates on Group resource to its members according to the model below.

class User(db.Model):
    id = db.Column(db.Integer, primary_key=True)
    email = db.Column(db.String(120), unique=True)
    group_id = db.Column(db.Integer, db.ForeignKey('group.id'))

class Group (db.Model):
    id = db.Column(db.Integer, primary_key=True)
    name = db.Column(db.String(140))
    members = db.relationship('User', backref='group')

goshasawicka avatar Aug 12 '16 21:08 goshasawicka

@lyschoening I managed to make it work, I'll send a PR once I figure the tests

odcinek avatar Aug 12 '16 21:08 odcinek

@odcinek Ok, thanks for doing this!

@Patyk That makes sense. Please note, if it is only a handful of group memberships I would recommend fetching them as ItemNeed when the identity is loaded.

lyschoening avatar Aug 13 '16 09:08 lyschoening

@odcinek Any update on your progress? I'd like to use this in my own project

jas32096 avatar Feb 21 '17 21:02 jas32096

I think I have really similar use case here. @odcinek @patyk did you manage to create something for this, or maybe you can share Your solution for that problem? I will greatly appreciate any insights on that :).

galuszkak avatar Mar 22 '17 09:03 galuszkak