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

[Fix] undefined index EuLoginUser group #14

Closed
wants to merge 1 commit into from

Conversation

pidera
Copy link

@pidera pidera commented Aug 28, 2020

This PR fixes an issue that throws a notice during login via de EU login portal.
An index groups is fetched from a received user, but this does not exists, and should be group instead.

This notice prevents a good login flow in a more strictly configured PHP environment.

Screenshot

eu_login_undefined_index

@drupol drupol mentioned this pull request Aug 28, 2020
1 task
@drupol
Copy link
Member

drupol commented Aug 28, 2020

Dear @pidera ,

Your proposal doesn't pass the tests yet but...

I've been working yesterday (see #15) on this issue and I will continue to work on it Monday, today I'm not available for this.

I just pushed the branch I was working on yesterday, could you please test it out and provide some feedback?

Thanks :-)

@pidera
Copy link
Author

pidera commented Aug 28, 2020

Dear @pidera ,

Your proposal doesn't pass the tests yet but...

I've been working yesterday (see #15) on this issue and I will continue to work on it Monday, today I'm not available for this.

I just pushed the branch I was working on yesterday, could you please test it out and provide some feedback?

Thanks :-)

Hi @drupol,

Thanks for the quick response. I have tested the branch (#15) in our application, and this seems to solve the issues we were experiencing!

As the issue is more than just group/groups naming, and the solution with the combined arrays and getter fallback returns seems pretty solid, as far as I can see, the solution in your PR is the way to go.

@drupol
Copy link
Member

drupol commented Aug 28, 2020

Good news already 👍

I'll continue my work on this on Monday.

@pidera pidera closed this Aug 28, 2020
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

Successfully merging this pull request may close these issues.

2 participants