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

Fulfill more plugin interfaces #8

Open
mauritsvanrees opened this issue Nov 15, 2022 · 5 comments
Open

Fulfill more plugin interfaces #8

mauritsvanrees opened this issue Nov 15, 2022 · 5 comments

Comments

@mauritsvanrees
Copy link
Member

Hi @mamico
A few years ago I wrote a PAS plugin called pas.plugins.headers. It takes a very different approach: let Apache or nginx handle the communication with SAML/OAUTH/oidc/whatever and let it add the relevant information in headers that it passes to Plone. The PAS plugin then simply looks at the headers and authenticates or authorises you based on them.
When I compare my plugin to yours, I notice a big difference: your plugin does not really function as a PAS plugin. :-) It is not registered for any plugin interfaces. This is not a problem of course, in local testing it works fine, and I hope to test it on the servers of a client the coming weeks. But I wonder if a few of of the features of my plugin are interesting here as well. I would be happy on behalf of Zest and our customer to contribute some code here, if initial testing is successful.

Let me check which plugin interfaces my plugin supports, and what that could mean in your plugin when they are activated. Actually, given the current flow of authentication, only one seems interesting:

  • Challenge plugin: redirect to acl_users/oidc/login. This would happen when you are anonymous and visit a page that requires authentication.

Well, something could be done with roles. I have default_roles = Member. In the ZMI this could be set to Manager, and then pas.plugins.memberpropertytogroup would no longer be needed in the setup that @erral describes in his blog post. Additionally if the oidc connection provides information about roles, I have settings for roles_header and allowed_roles that I could add. Then we would have to see if we want to read this once on login and save it in Plone (meaning you cannot un-assign a role automatically) or save it in the session and register the plugin as IRolesPlugin (and let it read the session on every request).

Is any of this interesting for you as contribution?

O, and a question: is your repo still the canonical repo? Or do you plan to fully move it over to the collective, which has a fork already?

@erral
Copy link
Member

erral commented Nov 15, 2022

I am interested in the role setting stuff indeed. I haven't had more time to continue with my authentication settings, but that would be interesting indeed.

@mamico
Copy link
Collaborator

mamico commented Nov 15, 2022

@mauritsvanrees, @erral all contributions are absolutely welcome.

And yes, I would be happy to promote the collective fork as canonical ... do you know how to do that?

BTW: I am working on documentation and fixes to make the product work smoothly with Microsoft Azure OIDC as well.

@mauritsvanrees
Copy link
Member Author

@mamico You could delete this repo. From the github docs: "When you delete a public repository, one of the existing public forks is chosen to be the new parent repository. All other repositories are forked off of this new parent and subsequent pull requests go to this new parent."
I guess that when you delete it, you can tell GitHub which other repo to use as default. But it could be random, it is not clear from the description.

Maybe try archiving the repo first.

@mauritsvanrees
Copy link
Member Author

Ah, or maybe first delete the collective fork, and then transfer your repo to the collective.
All these options are in Settings (General), near the bottom, in the "Danger Zone".

@mamico
Copy link
Collaborator

mamico commented Nov 21, 2022

Ah, or maybe first delete the collective fork, and then transfer your repo to the collective. All these options are in Settings (General), near the bottom, in the "Danger Zone".

@mauritsvanrees This worked perfectly. And the existing forks followed the change without any problems. Thx.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants