-
Notifications
You must be signed in to change notification settings - Fork 39
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
allow completely disabling built-in auth #267
Comments
hm thinking more about this: piccolo-admin uses piccolo-api so any auth decision made for piccolo-admin also has to work for piccolo-api (I'm just thinking loudly here). It would be awesome of both piccolo-api and piccolo-admin supported completely pluggable auth mechanisms (this would ofc not work with the login dialog in piccolo-admin). For example, we'd like to just pass an auth header jwt into the piccolo-admin app request. When piccolo-admin makes requests back to piccolo-api it should simply forward that same jwt to represent the user. We would of course handle login dialog etc outside of piccolo-api, we just need somewhere to stick a callable that could validate the jwt |
@trondhindenes Piccolo Admin uses session auth by default piccolo_admin/piccolo_admin/endpoints.py Lines 656 to 665 in a90ccf1
allow_unauthenticated parameter (False by default). If we set this parameter to True , we can access the admin as an unauthenticated (unknown) user.I don't know if it's even a good idea and if it's useful for you, but with this maybe we can bypass the default authentication in create_admin .Another option maybe would be to authorize the user using your authentication mechanism, then create a session (using SessionsBase) for that user and then use the default Piccolo Admin auth. Sorry if miss your point. |
nice, thanks! I'll do some more digging in a local fork of piccolo-admin and see if I can come up with an actual suggestion. |
okay, so this doesn't look to bad. I hacked piccolo-admin's endpoints file:
I can see that whenever I make a request to the frontend with an |
I can see the value in this. Some things might break - like the You might also be able to subclass class CustomBaseUser(Table, tablename="piccolo_user"):
@classmethod
async def login(cls, username: str, password: str) -> t.Optional[int]:
if await my_custom_login_endpoint(username=username, password=password):
user = await cls.objects().get(cls.username == username)
if user is None:
user = await cls.create_user(username=username, password=password, admin=True, active=True)
return user.id
return None
app = create_admin(auth_table=CustomBaseUser, ...) |
Nice, that's a good idea. Currently running with a "vendored" piccolo-admin fork, and everything (except the logour url as you mention) works beautifully. Here's the auth backend code:
Oauth-proxy forwards the current user's email in a header, so we simply pick that up and it as the user name. Not using piccolo-admin's user list or session table in any way So, we'll never really hit piccolo-admin's login endpoint since users will never get to piccolo-admin without being authenticated. But being able to supply a configurable logout url would be nice, we could plug that into the logout endpoint oauth-proxy supplies. If you want I'd be happy to write a proper guide with details on how to integrate piccolo-admin and oauth-proxy for piccolo's documentation (once we have the necessary bits and pieces in place ofc) |
I made a PR here #269 - it might not be complete enough for your liking. |
Hi,
We're looking to use piccolo-admin in a project, but we want to plug in our internal auth mechanisms, which we typically use together with oauth-proxy to wrap 3rd party apps in our auth layer.
Because of this, I'd like to use piccolo-admin but without any of the built-in authz/authn functionality. I'd like piccolo-admin to simply assume that every request comes from an admin users (we'll ofc handle authz/authn outside piccolo-admin).
I haven't yet looked at the code, but it would be interesting to hear of something like this is already supported or planned. From what I can see from the
create_admin
's parameters there doesn't seem to be any obvious way of achieving what I want today.The text was updated successfully, but these errors were encountered: