[sakai2-tcc] next steps for S3/Nakamura Hybrid and Notification Preferences votes

[Jean-Francois Leveque]

Dear L,


login

I've just had a quick look at 
http://jira.sakaiproject.org/secure/attachment/21877/SAK-17223+2.8.patch

It seems details are missing in your comments.

org.sakaiproject.login.filter.NakamuraAuthenticationFilter.principal
org.sakaiproject.login.filter.NakamuraAuthenticationFilter.hostname
org.sakaiproject.login.filter.NakamuraAuthenticationFilter.autoProvisionUser

are not mentioned in your comments.

Where is the documentation for those properties?

I think such as the following should be fixed before trying to build:
+	private static final Log LOG = LogFactory
+			.getLog(NakamuraAuthenticationFilter.class);

I'm not familiar with login but I think getSecret and 
getPrincipalLoggedIntoK2 should handle a null secret more carefully.

Looks like a null principal is dealt with in doFilter.

I can't find where the jsonObject added to the List returned by 
getPrincipalLoggedIntoK2 is used.

I'm afraid I cannot answer
// what about top.login = false ?
but I think this has to be answered before your changes get in.

Looks like there are still dependencies on 
org.sakaiproject.nakamura.utils. Thought they were gone.

Why is there a jdk15 classifier in the json-lib dependency?


providers

Now, I'm having a look at 
http://jira.sakaiproject.org/secure/attachment/21876/SAK-17222+2.8.patch.

Why are you using a different CONFIG_PREFIX instead of using hybrid 
shared configuration properties.

AFAICT, you're duplicating .principal, .hostname and .validateUrl.

You're also duplicating:
private static final String COOKIE_NAME = "SAKAI-TRACKING";
private static final String ANONYMOUS = "anonymous";

Why aren't those strings from configuration?

In authenticateUser, I think you should at least issue a WARNING when 
requested to authenticate with a null eid.

I think you should also issue a WARNING when asked to findUserByEmail 
with a null email or to getUser when edit is null.

I'm not knowledgeable enough about providers, but I think
+		// What is the best default?
in authenticateWithProviderFirst should be answered before your changes 
get in.

I wonder how unsupported getUsers should be handled.

I think my comment about handling a null secret in login also applies to 
provider.

Looks like there are still dependencies on 
org.sakaiproject.nakamura.utils. Thought they were gone.


Hybrid

As for the main hybrid, I don't know for sure if you've answered AZ's 
questions already but my small knowledge makes me think you didn't.


Thanks, J-F

Speelmon, Lance Day a écrit :
> Dear TCC,
> 
> I have commented on all the reported concerns and issues re: hybrid. 
>  You will find them inline:  http://confluence.sakaiproject.org//x/GoQ2B
> 
> Thanks, L
> 
> 
> Lance Speelmon
> Scholarly Technologist
> 
> On Oct 8, 2010, at 12:45 PM, Seth Theriault wrote:
> 
>> Hello,
>>
>> As you know, both the S3/Nakamura Hybrid and Notification
>> Preferences proposals received -1 votes in the intial rounds.
>> They are now subject to a roll-call override vote.
>>
>> A number of people asked questions or made alternative
>> suggestions. I think it's important for Lance and Chris to be
>> able to respond to these questions so that the TCC can be fully
>> informed for the roll-call voting.
>>
>> In addition, we have a new tag being cut next week and the
>> inclusion decisions need to be finalized.
>>
>> Therefore, I think it is prudent to do the following:
>>
>> 1) Allow Lance and Chris to answer your questions and conclude
>> any follow-up discussion by 23:59 UTC, on this TUESDAY, 12
>> October 2010.
>>
>> 2) Open roll-call votes on these proposals for a short period to
>> end at 23:59 UTC, on this WEDNESDAY, 13 October 2010, with a "no
>> comments" rule for the voting period.
>>
>> 3) Delay cutting the 2nd alpha tag to Thursday, 14 October 2010,
>> so that all TCC-approved 2.8 tools can be included.
>>
>> Again, I am trying to balance the a pragmatic approach with the
>> need to solidify our 2.8 offering and be sure it can be
>> thoroughly tested.
>>
>> If you have objections to this plan, speak now.
>>
>> Seth