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 SSO access #176

Open
wants to merge 1 commit into
base: testlink_1_9
Choose a base branch
from

Conversation

Dawuid
Copy link

@Dawuid Dawuid commented Sep 30, 2018

Some old code about SSO access has not been updated

Some old code about SSO access has not been updated
@fmancardi
Copy link
Contributor

Please provide better macro details of changes, in order to make simpler the code review
thanks

@sglder
Copy link

sglder commented Jul 22, 2021

Hi @fmancardi. Could you please reconsider merging this pull request or applying the fix in another way? I ran into the same bug 3 years later, just to find out a fix was already proposed.

As for the changes: In b9064c0 you added some code to doSessionSetUp in lib/functions/doAuthorize.php. In the process $userObj->getSecurityCookie() was changed to $user->getSecurityCookie() which seems to have been an oversight. $user gets passed to the function in doSSOWebServerVar line 387, however the parameter is called userObj. $user is undefinde in the scope of doSessionSetUp.

The change to line 443 seems gratuitous, since setUserSessionFromObj does the same thing as the proposed change except for setting the locale (which seems to happen another way, because the language gets selected correctly, for me anyway).

@fmancardi
Copy link
Contributor

Hi sglder

  1. As you can see I've requested clarifications that were never received then my choice was no action at all
  2. I'm going to fix the typo error regarding $user -> $userObject, but because I've no environment to test the SSO I will need your help:
    2.1 is this changed enough?

change will be done on branch testlink_1_9_20_fixed.

regards

@fmancardi
Copy link
Contributor

Just commited

@sglder
Copy link

sglder commented Jul 24, 2021

Thanks for the applying the fix.

  1. I'm going to fix the typo error regarding $user -> $userObject, but because I've no environment to test the SSO I will need your help:
    2.1 is this changed enough?

Yes, with the fix the method WEBSERVER_VAR seems to work fine.

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.

3 participants