Dave wrote:
On Thu, Jul 31, 2008 at 2:12 PM, Allen Gilliland
<[EMAIL PROTECTED]> wrote:
I don't mean to be difficult, but I'm still not a fan of all the OpenID
specific methods. The couple things about this proposal which still don't
seem right to me are ...
No need to apologize. I really appreciate that you took the time to
read and respond to the proposal.
1. Why do we need to support multiple OpenID urls per user? Nobody has
answered that question yet and it seems to me that it's important. It's a
pretty significant simplification to the implementation if we only support a
single OpenID url per user account, so unless there is a very specific
reason we think multiple are required then I don't see why we would do that.
If it really is necessary then fine, it just seems like a relevant question
which nobody has answered.
I do not think this is an absolute requirement at all. It is a "nice
to have" feature only.
If it's not required then I would opt for the easy route and just
support a single OpenID per user.
2. The point of the generic support for user attributes was to avoid methods
like the OpenID specific ones proposed for the UserManager. Those methods
should be generic methods for user attributes, not OpenID specific, such as
...
getUserByAttribute("openid.url", "attribute.value");
getUser().getAttribute("openid.url");
getUser().setAttribute("openid.url", "attribute.value");
I agree, that is a much better approach. I don't like having OpenID
specific methods either.
3. I don't know why it would be necessary to modify the UIAction class at
all and even the modifications to the Register class should be non-OpenID
specific. The way the prepopulating of the Register action should work is
that *if* the client is known then the result of getAuthenticatedUser()
should return a User object representing them which can then be used to
prepopulate the bean used by the action.
I agree here too.
And note, in the latest patch from Tatyana, there is no change to UIAction.
https://issues.apache.org/roller/browse/ROL-1733
I believe there will have to be some OpenID specific things in the
login page, but we I think we can avoid OpenID specific code almost
everywhere else.
Yeah, that sounds right to me. You'll definitely need some changes to
the Login struts2 action and jsp.
The current Register action implementation isn't quite right yet which is
why some changes are necessary, but if it was right then there should really
be no need to modify that code at all. The current code is doing some funky
usingSSO/fromSSO code which doesn't need to be at that place in the code.
That code can be pushed up into the RollerSession code so that if the
client is identifiable via SSO or any other way then we can access a User
object that represents them.
I'm not sure I understand what you mean here. What logic are
suggesting needs to be moved to RollerSession?
in Register.execute(), the try/catch block which looks for a User object
via CustomUserRegistery.
That block should be replaced with a simple bit of code like ...
if(getAuthenticatedUser() != null) {
getBean().copyFrom(getAuthenticatedUser());
}
For that to work we need to ensure that UIAction.getAuthenticatedUser()
returns a valid User object in all cases where the client is authentic,
not just in cases where the client has a local session.
This is effectively the same situation I ran into when doing comment
authentication. We need to support the notion that a client can be
authentic even if they don't have a local session and aren't directly
part of the local Roller db.
We already do the right thing in the UIActionInterceptor, which means we
should have the authenticated User object available to all s2 actions ...
RollerSession rses = RollerSession.getRollerSession(request);
if(rses != null) {
theAction.setAuthenticatedUser(rses.getAuthenticatedUser());
}
So really all we need to do is move that original bit of code that is in
the try/catch block of the Register.execute() method and move it into
the RollerSession code so that when you are trying to get a User from
RollerSession it automatically consults SSO.
-- Allen
sorry to be tough, it's just that if we don't do this right on the first
pass then we'll have to go back and redo it later, which is more work :/
Thanks, this is good feedback.
- Dave
Tatyana has updated her OpenID for Roller proposal here:
http://cwiki.apache.org/confluence/x/zVAB
