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

Auth capability 'login' missing #2173

Closed
wants to merge 2 commits into from
Closed

Auth capability 'login' missing #2173

wants to merge 2 commits into from

Conversation

janw
Copy link

@janw janw commented Nov 24, 2017

In regards to #797, this PR is supposed to add some sort of 'login' capability to an Auth Plugin to allow for custom Login Forms to be presented instead of the default login, that may or may not be used by the plugin.

This is a work in progress. Please don't judge

@janw janw changed the title Login auth capabilty Auth capability 'login' missing Nov 24, 2017
@splitbrain splitbrain added this to the Greebo milestone Nov 26, 2017
Copy link
Collaborator

@splitbrain splitbrain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain the new flow a bit more? What's the expected behavior after your change?

$form->addElement(form_makeButton('submit', '', $lang['btn_login']));
$form->endFieldset();
if(actionOK('login')){
print tpl_actionlink('login','','','',true);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this change. When the login action is enabled you show a link to the login form otherwise you show the form itself?

@splitbrain
Copy link
Collaborator

@JanWH are you still working on this?

@janw
Copy link
Author

janw commented Jan 3, 2018

@splitbrain Sorry for not replying earlier, as I am currently on vacation in this regards a work project of mine. I will get back to you early next week!

@micgro42
Copy link
Collaborator

I looked into rewriting and thus replacing this pull request. However, I'm not sure I agree that it is useful/necessary. Plugins can already modify/remove the login form. The oauth plugin is doing exactly that with the HTML_LOGINFORM_OUTPUT event.
Adding a cando['login'] would effectively prevent plugins from handling the login action itself, not only the preventing the form to be displayed. Is that the desired functionality?

I'm somewhat confused^^

@janw
Copy link
Author

janw commented Apr 4, 2018

Sorry for the late reply. Thanks @micgro42, I did not discover the HTML_LOGINFORM_OUTPUT event. Indeed, I wanted the plugin to handle the login action, for example in the case of a single OAuth provider, to redirect directly to the login page of said provider. In the light of HTML_LOGINFORM_OUTPUT this was a stupid idea, I used it to figure out a proper way to build our authentication.

I am sorry for wasting everybody's time. 😞

@janw janw closed this Apr 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants