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

Move frpm 3.24 to 3.3.2 don't work #648

Closed
jo49bimpf opened this Issue Jul 17, 2017 · 26 comments

Comments

Projects
None yet
6 participants
@jo49bimpf

jo49bimpf commented Jul 17, 2017

Hello,
I'm moving from 3.2.4 to 3.3.2. But the application dies. With debugging I see, that the AutManager returns code 401. So I think theres is an problem with dthe autehtication/authorization.

Is there any change?
WQhat can I do to find/fix it?

Thanks.
Juergen

@Airakath

This comment has been minimized.

Airakath commented Jul 17, 2017

Hello ,

I know there is a change, but I do not remember ...
Can you link me your code so that I compare to mine?

@Airakath

This comment has been minimized.

Airakath commented Jul 17, 2017

I need your application.xml and authManager

@jo49bimpf

This comment has been minimized.

jo49bimpf commented Jul 17, 2017

Hello,

thnaks for your help. Here are the files, I've changed only the ip's, dbnames and credentials.

Desktop.zip

@Airakath

This comment has been minimized.

Airakath commented Jul 17, 2017

I read your code, I found some differences.

I use:

Prado::using ('System.Security.TDbUserManager');

and

class userManager extends TDbUser

and you:

Prado::using('System.Security.IUserManager');

and

class TAuthManager extends TModule

Try to change

@Airakath

This comment has been minimized.

@ctrlaltca

This comment has been minimized.

Member

ctrlaltca commented Jul 17, 2017

Hello, I'm moving from 3.2.4 to 3.3.2

Hi, you may want to have a read at http://www.pradoframework.net/demos/quickstart/index.php?page=GettingStarted.Upgrading32 if you didn't already

But the application dies. With debugging I see, that the AutManager returns code 401

I'd start reading the error_log of the webserver, where you should find the exact place and error that is causing the page to fail. I see you also enabled file logging in protected/log/Leutek.log, that file can contain some useful informations, too

@jo49bimpf

This comment has been minimized.

jo49bimpf commented Jul 17, 2017

Hi,

I've read this manual and I've looked to the error_logs. Unfortunately there no entries for this behavior (in the doc and in teh log). I'll try the hints from Airakath at thursday because I'm out for the next two days.
I'll comennt the results.

I think, the authmanager will show an 401-error, but the page crashes, too. I see only a white browser-window after starting the index.php. And an execptuion won't be thrown.

Thanks and best regards
Juergen

@jo49bimpf

This comment has been minimized.

jo49bimpf commented Jul 17, 2017

So I've tried the code an get the follwing execption:

TPhpErrorException
Description

[Error] Class userManager contains 2 abstract methods and must therefore be declared abstract or implement the remaining methods (TDbUser::validateUser, TDbUser::createUser) (@line 457 in file ......\prado\prado-3.3.2.r8978869\framework\Security\TAuthManager.php).

But I don't understand the code missmatch because I used the stable-version of prado.

Do I have to change the settings.xml for the new version?
With 3.2.4 it still works ...

Thanks alot
Juergen

@Airakath

This comment has been minimized.

Airakath commented Jul 18, 2017

Try this

    public function createUser($username)
	{	
		$UtilisateurRecord = null;

		$UtilisateurRecord = UtilisateurRecord::finder()->findBy_uti_Login_or_uti_Email($username,$username);
	
        if($UtilisateurRecord instanceof UtilisateurRecord)
		{
            $user = new userManager($this->Manager);
            $user->Name = $UtilisateurRecord->uti_Login."#".$UtilisateurRecord->uti_ID;
			//explode("#", $user->Name)[0] récupe username
			//explode("#", $user->Name)[1] récupe id

            $user->roles = $UtilisateurRecord->rang->ran_Niveau;

            $user->IsGuest=false;
            return $user;
        }   
    }


	/**
	 * Vérifie que le nom d'utilisateur et son mot de passe sont correct.
	 * Cette méthode est requise par TDbUser.
	 * @param string le nom de l'utilisateur
	 * @param string le mot de passe
	 * @return boolean en fonction de la validité de la vérification.
	 */

    public function validateUser($mylogin, $mypass)
	{

		return (UtilisateurRecord::finder()->find('(uti_Login = ? or uti_Email = ?)  AND uti_MotDePasse = ? AND uti_Confirmer = 1', $mylogin,$mylogin, md5($mypass)) !== null);
        
    }
@LCSKJ

This comment has been minimized.

Member

LCSKJ commented Jul 18, 2017

If the TAuthManager class you provided in the zip file is different from PRADO's one you already made a mistake in the first place. Never touch any of PRADO's classes directly, either use them as they are or implement/derive your own class. Tampering the base classes won't let you update anymore - at least not easily.

Looking at the provided application.xml you are using the TAuthManager class for authorization, which is using the TDbUserManager class for user handling. This is perfectly fine, none of those classes need to be changed anywhere, just use them as provided unless you really need them to behave different.
The TDbUserManager also needs a user factory class which needs to derive from TDbUser, this is what @Airakath's point was. The error message you got is because TDbUser is abstract and therefor needs your class to implement both abstract methods validateUser() and createUser() which userManager misses.

In your application.xml the attribute UserClass="WM_User" tells the TDbUserManager that you are providing a class WM_User derived from TDbUser which is implementing those two missing abstract methods.

So all you have to do to make it work with your current application.xml is to provide a user class named WM_User for validating and creating users ... something starting with this:

class WM_User extends TDbUser {
  public function validateUser($username,$password) {
    // your validation goes here, maybe select user from db by username, compare passwords and return either true or false
  }
  public function createUser($username) {
    // create your user instance here, although it only needs to implement IUser your should return a WM_User instance
  }
}
@jo49bimpf

This comment has been minimized.

jo49bimpf commented Jul 20, 2017

Hello again,

I've this class WM_User with the mentioned functions.

I've done some debugging. The page stopps TAuthorizationRule.php in the function isUserAllowed:

public function isUserAllowed($user,$verb,$ip)
	{
		if($user instanceof IUser)
		{
			$verb=strtolower(trim($verb));
			foreach($this as $rule)
			{
				if(($decision=$rule->isUserAllowed($user,$verb,$ip))!==0)
					return ($decision>0);
			}
			return true;
		}
		else
			return false;
	}

in this function I#ll get -1 for $decision which causes the Auth-Error 401 in this function from TAuthmanager:

	{
		$application=$this->getApplication();
	if(!$application->getAuthorizationRules()->isUserAllowed($application->getUser(),$application->getRequest()->getRequestType(),$application->getRequest()->getUserHostAddress()))
		{
			$application->getResponse()->setStatusCode(401);
			$application->completeRequest();
		}
	}

But why in 3.3.2 an not in 3.2.4, which uses the same protected and the same dta?

@Airakath

This comment has been minimized.

Airakath commented Jul 20, 2017

You need the two functions that I asked you and @LCSKJ that spoke to you

I myself, when I made the migration I made modifications

@jo49bimpf

This comment has been minimized.

jo49bimpf commented Jul 21, 2017

I've these functions in my WMUser.php.

If I comment this lines un TApplication->run()
//if($this->_requestCompleted)
//break;

the application will start welcome.php, but there it will struggle again.
I see, that the event "onAuthorizationComplete" won't come (and there the TApplication->run() normally ends).
But why?
Until this point, everything looks identical between version 3.2.4 and 3.3.2.

@jo49bimpf

This comment has been minimized.

jo49bimpf commented Jul 21, 2017

Somesthing more:
from the function "public function onAuthorize($param)" I get from the subfunction isAllowed (From TAuthmangerRules) the user back:

WM_User Object ( [_connection:TDbUser:private] => [_state:TUser:private] => Array ( [Name] => Guest [IsGuest] => 1 ) [_stateChanged:TUser:private] => 1 [_manager:TUser:private] => TDbUserManager Object ( [_connID:TDbUserManager:private] => [_conn:TDbUserManager:private] => [_guestName:TDbUserManager:private] => Guest [_userClass:TDbUserManager:private] => WM_User [_userFactory:TDbUserManager:private] => WM_User Object ( [_connection:TDbUser:private] => [_state:TUser:private] => Array ( [Name] => Guest ) [_stateChanged:TUser:private] => 1 [_manager:TUser:private] => TDbUserManager Object RECURSION [_e:TComponent:private] => Array ( ) [_listeningenabled:TComponent:private] => [_behaviorsenabled:TComponent:private] => 1 [_m:TComponent:private] => ) [_id:TModule:private] => users [_e:TComponent:private] => Array ( ) [_listeningenabled:TComponent:private] => [_behaviorsenabled:TComponent:private] => 1 [_m:TComponent:private] => ) [_e:TComponent:private] => Array ( ) [_listeningenabled:TComponent:private] => [_behaviorsenabled:TComponent:private] => 1 [_m:TComponent:private] => )
rule -->TAuthorizationRule Object ( [_action:TAuthorizationRule:private] => deny [_users:TAuthorizationRule:private] => Array ( ) [_roles:TAuthorizationRule:private] => Array ( [0] => * ) [_verb:TAuthorizationRule:private] => * [_ipRules:TAuthorizationRule:private] => Array ( [0] => * ) [_everyone:TAuthorizationRule:private] => [_guest:TAuthorizationRule:private] => 1 [_authenticated:TAuthorizationRule:private] => [_e:TComponent:private] => Array ( ) [_listeningenabled:TComponent:private] => [_behaviorsenabled:TComponent:private] => 1 [_m:TComponent:private] => )

@frkinta

This comment has been minimized.

Contributor

frkinta commented Jul 21, 2017

There were many good demos for starter here (http://www.pradoframework.net/site/demos/) including login example in 'Personal website' but not working now

EDIT: Oops. I finally found the link http://www.pradoframework.net/demos/personal/. You can find an example of authentication here :
https://github.com/pradosoft/prado-demos/blob/master/personal/protected/Common/LoginPortlet.php
https://github.com/pradosoft/prado-demos/blob/master/personal/protected/Common/LoginPortlet.tpl

@jo49bimpf

This comment has been minimized.

jo49bimpf commented Jul 27, 2017

yes, but my script doesn't come to this point. It exits long before the authentication, in my case in WM_User is done. With Prado 3.2.4 all works fine. But not with 3.3.2. That's the problem. I want to upgrade, but I can't

@jo49bimpf

This comment has been minimized.

jo49bimpf commented Jul 27, 2017

Bingo, got it!!!!

sendHTTPHeader is the problem.

In the old version it sends
"HTTP/1.1 401 Unauthorized, true, 401"

and in the 3.3.2-version

"Status: 401 Unauthorized, true,401"

This is the verion from 3.3.2 (which is not working in my environment):

/**
 * Send the HTTP header with the status code (defaults to 200) and status reason (defaults to OK)
 */
protected function sendHttpHeader()
{
	$protocol=$this->getRequest()->getHttpProtocolVersion();
		
	if($this->getRequest()->getHttpProtocolVersion() === null)
		$protocol='HTTP/1.1';

	$phpSapiName = substr(php_sapi_name(), 0, 3);
	$cgi = $phpSapiName == 'cgi' || $phpSapiName == 'fpm';
	
	
	header(($cgi ? 'Status:' : $protocol).' '.$this->_status.' '.$this->_reason, true, TPropertyValue::ensureInteger($this->_status));

	$this->_httpHeaderSent = true;
}

And this is the version of 3.2.4, which ist working fine (even if I copie it into the 3.3.2-php-module)

protected function sendHttpHeader()
{
	
	if (($version=$this->getRequest()->getHttpProtocolVersion())==='')
		header (' ', true, $this->_status);
	else
		header($version.' '.$this->_status.' '.$this->_reason, true, $this->_status);

	$this->_httpHeaderSent = true;
}

Is there an issue with windows?
My apache and php ist runnin on a windows-server.
Here are some infos of the system:

PHP Version 5.6.30
System Windows NT D100SAV-EMDB-GR 6.3 build 9600 (Windows Server 2012 R2 Standard Edition) i586
Build Date Jan 18 2017 19:41:45
Compiler MSVC11 (Visual C++ 2012)
Architecture x86
Configure Command cscript /nologo configure.js "--enable-snapshot-build" "--enable-debug-pack" "--disable-zts" "--disable-isapi" "--disable-nsapi" "--without-mssql" "--without-pdo-mssql" "--without-pi3web" "--with-pdo-oci=c:\php-sdk\oracle\x86\instantclient_12_1\sdk,shared" "--with-oci8-12c=c:\php-sdk\oracle\x86\instantclient_12_1\sdk,shared" "--with-enchant=shared" "--enable-object-out-dir=../obj/" "--enable-com-dotnet=shared" "--with-mcrypt=static" "--without-analyzer" "--with-pgo"
Server API CGI/FastCGI

And the apache is a:
[VERSION_APACHE]

2.4.25 - 20.01.2017

"MAJOR"="2"
"MINOR"="4"
"PATCH"="25"
"BUILD_HOTFIX"="0"
"DATE_DAY"="20"
"DATE_MONTH"="1"
"DATE_YEAR"="2017"
"PLATFORM"="win64"

Hope, someone can help me. Thanks a lot.

@ctrlaltca

This comment has been minimized.

Member

ctrlaltca commented Jul 27, 2017

looks like we implemented a workaround for a php bug in cgi mode (and extended it to fastcgi mode in prado 3.3: 6258436), but it's not needed anymore: https://stackoverflow.com/questions/8828275/still-necessary-to-use-status-404-not-found-for-fcgi

ctrlaltca added a commit that referenced this issue Jul 27, 2017

@ctrlaltca ctrlaltca closed this in 91317ba Jul 27, 2017

@ctrlaltca ctrlaltca reopened this Jul 27, 2017

@ctrlaltca

This comment has been minimized.

Member

ctrlaltca commented Jul 27, 2017

So, i've just committed a fix in 93ffb3e
Please have a try.

@ctrlaltca

This comment has been minimized.

Member

ctrlaltca commented Aug 3, 2017

@jo49bimpf can you please confirm if the fix works for you?

@jo49bimpf

This comment has been minimized.

jo49bimpf commented Aug 3, 2017

@ctrlalta: sorry for the delay, I'd to fix a hughe customer-problem. So today I've tested the fix and it will work for me out of the box.

Thanks a lot and best regards
Juergen

@ctrlaltca

This comment has been minimized.

Member

ctrlaltca commented Aug 3, 2017

@jo49bimpf Thank you for your feedback!

@ctrlaltca ctrlaltca closed this Aug 3, 2017

@abienvenu

This comment has been minimized.

abienvenu commented Jan 25, 2018

I find the workaround 6258436 quiet harmful. When Prado runs with PHP-FPM, it may send replies with Location: header set, but with a HTTP status of 200 or 401. The redirection is then ignored by the browser.
Fix 93ffb3e is great, thanks @ctrlaltca ! Could you please make a packagist release for it ?

@ctrlaltca

This comment has been minimized.

Member

ctrlaltca commented Jan 25, 2018

@abienvenu a release for branch 3.3? Sure

@abienvenu

This comment has been minimized.

abienvenu commented Jan 25, 2018

Yes, this would be awesome. At this moment, the latest pradosoft/prado release is 3.3.2 on packagist, and we miss your fix :)

@ctrlaltca

This comment has been minimized.

Member

ctrlaltca commented Jan 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment