Verified the fix to check for possible regression of GERONIMO-2266 and
GERONIMO-2267. No regression.
Vamsi
On 8/14/07, David Jencks <david_jencks@yahoo.com> wrote:
>
>
> On Aug 13, 2007, at 9:33 PM, Kevan Miller wrote:
>
> > I'd also make the following changes.
> >
> > Comments? The Test updates are optional, but may as well clean
> > those up.
>
> I agree with all of these except the
> CallerIdentityPasswordCredentialLoginModule. I think there are two
> choices:
>
> 1. throw a FailedLoginException() in login if resourcePrincipalName
> == null || userName == null || password == null
> 2. do nothing except return false in commit if resourcePrincipalName
> == null || userName == null || password == null.
>
> I lean towards the second. Normally we will already have verified
> that all the necessary info is present in a previous login module.
> If its not present, then if we don't add a PasswordCredential then I
> believe the authentication for the connector will use whatever
> default is configured which seems to me like a reasonable fallback.
>
> thanks
> david jencks
>
> >
> > --kevan
> >
> > coltrane:~/geronimo/server/trunk kevan$ svn diff
> > Index: testsuite/enterprise-testsuite/sec-tests/sec-ejb/src/main/
> > java/org/apache/geronimo/itest/TestLoginModule.java
> > ===================================================================
> > --- testsuite/enterprise-testsuite/sec-tests/sec-ejb/src/main/java/
> > org/apache/geronimo/itest/TestLoginModule.java (revision 565610)
> > +++ testsuite/enterprise-testsuite/sec-tests/sec-ejb/src/main/java/
> > org/apache/geronimo/itest/TestLoginModule.java (working copy)
> > @@ -69,7 +69,10 @@
> > }
> > user = ((NameCallback)callbacks[0]).getName();
> > String password = new String(((PasswordCallback)callbacks
> > [1]).getPassword());
> > - return user.equals(password) && users.contains(user);
> > + if (user.equals(password) && users.contains(user)) {
> > + return true;
> > + }
> > + throw new LoginException();
> > }
> > public boolean commit() throws LoginException {
> > Index: modules/geronimo-security/src/main/java/org/apache/geronimo/
> > security/jaas/UPCredentialLoginModule.java
> > ===================================================================
> > --- modules/geronimo-security/src/main/java/org/apache/geronimo/
> > security/jaas/UPCredentialLoginModule.java (revision 565610)
> > +++ modules/geronimo-security/src/main/java/org/apache/geronimo/
> > security/jaas/UPCredentialLoginModule.java (working copy)
> > @@ -72,7 +72,7 @@
> > String username = ((NameCallback) callbacks[0]).getName();
> > char[] password = ((PasswordCallback) callbacks
> > [1]).getPassword();
> > - if (username == null || password == null) return true;
> > + if (username == null || password == null) return false;
> > upCredential = new UsernamePasswordCredential(username,
> > password);
> > Index: modules/geronimo-connector/src/main/java/org/apache/geronimo/
> > connector/outbound/security/
> > CallerIdentityPasswordCredentialLoginModule.java
> > ===================================================================
> > --- modules/geronimo-connector/src/main/java/org/apache/geronimo/
> > connector/outbound/security/
> > CallerIdentityPasswordCredentialLoginModule.java (revision 565610)
> > +++ modules/geronimo-connector/src/main/java/org/apache/geronimo/
> > connector/outbound/security/
> > CallerIdentityPasswordCredentialLoginModule.java (working copy)
> > @@ -81,10 +81,13 @@
> > resourcePrincipalName = ((NameCallback) callbacks
> > [0]).getName();
> > userName = ((NameCallback) callbacks[0]).getName();
> > password = ((PasswordCallback) callbacks[1]).getPassword();
> > - return resourcePrincipalName != null && userName != null
> > && password != null;
> > + return false;
> > }
> > public boolean commit() throws LoginException {
> > + if (resourcePrincipalName == null || userName == null ||
> > password == null) {
> > + throw new LoginException();
> > + }
> > subject.getPrincipals().add(new ResourcePrincipal
> > (resourcePrincipalName));
> > PasswordCredential passwordCredential = new
> > PasswordCredential(userName, password);
> > passwordCredential.setManagedConnectionFactory
> > (managedConnectionFactory);
> > Index: modules/geronimo-jmx-remoting/src/test/java/org/apache/
> > geronimo/jmxremoting/AuthenticatorTest.java
> > ===================================================================
> > --- modules/geronimo-jmx-remoting/src/test/java/org/apache/geronimo/
> > jmxremoting/AuthenticatorTest.java (revision 565610)
> > +++ modules/geronimo-jmx-remoting/src/test/java/org/apache/geronimo/
> > jmxremoting/AuthenticatorTest.java (working copy)
> > @@ -112,7 +112,10 @@
> > if (password == null) {
> > throw new FailedLoginException();
> > }
> > - return password.equals(new String
> > (passwordCallback.getPassword()));
> > + if (password.equals(new String
> > (passwordCallback.getPassword()))) {
> > + return true;
> > + }
> > + throw new FailedLoginException();
> > } catch (java.io.IOException e) {
> > throw new FailedLoginException();
> > } catch (UnsupportedCallbackException e) {
> >
>
>
|