#41 Ticket #191, populate user attributes when authentication is external to Ipsilon
Merged by puiterwijk. Opened by jdennis.
jdennis/ipsilon apache-login  into  master

Download 41.patch
no initial comment

FYI: all unit tests continue to pass with this patch. This is needed for Openstack ECP, Nathan would like to get the patches in place so we can create a patch set for RHEL 7.2 z stream.

Thanks for the info.
Could you please rebase your patch against the current master and push that?
As soon as you have done that I'll review it.

This will re-initialize all the info plugins all over again.
It would be better if you just grab the info from the root.login.info.

Why do you return this, if you never use the return value?

Okay, the rebase is a clear rebase, and other than my two comments it looks good.
Please answer on my comments and fix at least the second one.

How do I know root.login.info has been initialized? Maybe the Info class needs a factory method.

How do I know root.login.info has been initialized already? Maybe the Info class needs a factory method.

If you set self.info inside init_idp, you are certain that: 1. it gets set before any requests come in, and 2. the framework itself has finished initializing at least the basic structures and pointers.

Because this is where the session object is created, if you need access to it then where do you get it from unless it's returned from this method? There is no harm in returning a value even if the return value is not currently utilized.

Fair enough, I was just being curious.

If you set self.info inside init_idp, you are certain that: 1. it gets set before any requests come in, and 2. the framework itself has finished initializing at least the basic structures and pointers.

I don't think your suggestion is going to work Patrick. I tried to implement it but the class hierarchy doesn't allow for it, or at least as far as I can tell. Here is the problem, SSO_SOAP is derived from AuthenticateRequest which is derived from ProviderPageBase. init_idp is a method of the class IdpProvider which is derived from ProviderBase. ProviderBase. ProviderBase has the member _root which is the root that has login hung off it. But where I need access to the Info object is in a class (SSO_SOAP) that is derived from ProviderPageBase and there is no access to the root there.

Unless you can think of another way to access root.info from withing SSO_SOAP (I couldn't find a way) then I think making Info a singleton and getting it via a factory is the best solution, or I'm open to another idea. Thoughts?

Patch has been reworked to obtain a reference to login.info instead of instantiating a new Info object.

Looks good to me.

Metadata