Refactor the SSH session state

We want to split the session state apart from the actual connection
so we can implement a "set uid" feature later, where the user
running a command may not match the original authentication.

Change-Id: I0c9d31b4f5f04849e1c4a171243f0f376056c2c8
Signed-off-by: Shawn O. Pearce <sop@google.com>

gerrit-server/src/main/java/com/google/gerrit/server/IdentifiedUser.java

@@ -69,8 +69,13 @@ public class IdentifiedUser extends CurrentUser {
     }
 
     public IdentifiedUser create(final Account.Id id) {
-      return new IdentifiedUser(AccessPath.UNKNOWN, authConfig, canonicalUrl,
+      return create(AccessPath.UNKNOWN, null, id);
-          realm, accountCache, null, null, id);
+    }
+
+    public IdentifiedUser create(AccessPath accessPath,
+        Provider<SocketAddress> remotePeerProvider, Account.Id id) {
+      return new IdentifiedUser(accessPath, authConfig, canonicalUrl, realm,
+          accountCache, remotePeerProvider, null, id);
     }
   }
 

gerrit-sshd/src/main/java/com/google/gerrit/sshd/BaseCommand.java

@@ -14,20 +14,22 @@
 
 package com.google.gerrit.sshd;
 
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.RequestCleanup;
 import com.google.gerrit.server.git.WorkQueue;
 import com.google.gerrit.server.git.WorkQueue.CancelableRunnable;
 import com.google.gerrit.server.project.NoSuchChangeException;
 import com.google.gerrit.server.project.NoSuchProjectException;
-import com.google.gerrit.sshd.SshScopes.Context;
+import com.google.gerrit.sshd.SshScope.Context;
 import com.google.gerrit.util.cli.CmdLineParser;
 import com.google.inject.Inject;
+import com.google.inject.Provider;
 
 import org.apache.sshd.common.SshException;
 import org.apache.sshd.server.Command;
 import org.apache.sshd.server.Environment;
 import org.apache.sshd.server.ExitCallback;
-import org.apache.sshd.server.session.ServerSession;
 import org.kohsuke.args4j.Argument;
 import org.kohsuke.args4j.CmdLineException;
 import org.kohsuke.args4j.Option;
@@ -74,6 +76,12 @@ public abstract class BaseCommand implements Command {
   @CommandExecutor
   private WorkQueue.Executor executor;
 
+  @Inject
+  private Provider<CurrentUser> userProvider;
+
+  @Inject
+  private Provider<SshScope.Context> contextProvider;
+
   /** The task, as scheduled on a worker thread. */
   private Future<?> task;
 
@@ -324,14 +332,17 @@ public abstract class BaseCommand implements Command {
 
     if (e instanceof UnloggedFailure) {
     } else {
-      final ServerSession session = SshScopes.getContext().session;
       final StringBuilder m = new StringBuilder();
-      m.append("Internal server error (");
+      m.append("Internal server error");
-      m.append("user ");
+      if (userProvider.get() instanceof IdentifiedUser) {
-      m.append(session.getUsername());
+        final IdentifiedUser u = (IdentifiedUser) userProvider.get();
+        m.append(" (user ");
+        m.append(u.getAccount().getUserName());
         m.append(" account ");
-      m.append(session.getAttribute(SshUtil.CURRENT_ACCOUNT));
+        m.append(u.getAccountId());
-      m.append(") during ");
+        m.append(")");
+      }
+      m.append(" during ");
       m.append(getFullCommandLine());
       log.error(m.toString(), e);
     }
@@ -376,19 +387,28 @@ public abstract class BaseCommand implements Command {
   private final class TaskThunk implements CancelableRunnable {
     private final CommandRunnable thunk;
     private final Context context;
+    private final String taskName;
 
     private TaskThunk(final CommandRunnable thunk) {
       this.thunk = thunk;
-      this.context = SshScopes.getContext();
+      this.context = contextProvider.get();
+
+      StringBuilder m = new StringBuilder();
+      m.append(getFullCommandLine());
+      if (userProvider.get() instanceof IdentifiedUser) {
+        IdentifiedUser u = (IdentifiedUser) userProvider.get();
+        m.append(" (" + u.getAccount().getUserName() + ")");
+      }
+      this.taskName = m.toString();
     }
 
     @Override
     public void cancel() {
+      final Context old = SshScope.set(context);
       try {
-        SshScopes.current.set(context);
         onExit(STATUS_CANCEL);
       } finally {
-        SshScopes.current.set(null);
+        SshScope.set(old);
       }
     }
 
@@ -397,10 +417,10 @@ public abstract class BaseCommand implements Command {
       final Thread thisThread = Thread.currentThread();
       final String thisName = thisThread.getName();
       int rc = 0;
+      final Context old = SshScope.set(context);
       try {
         context.started = System.currentTimeMillis();
-        thisThread.setName("SSH " + toString());
+        thisThread.setName("SSH " + taskName);
-        SshScopes.current.set(context);
         try {
           thunk.run();
         } catch (NoSuchProjectException e) {
@@ -424,7 +444,7 @@ public abstract class BaseCommand implements Command {
         try {
           onExit(rc);
         } finally {
-          SshScopes.current.set(null);
+          SshScope.set(old);
           thisThread.setName(thisName);
         }
       }
@@ -432,9 +452,7 @@ public abstract class BaseCommand implements Command {
 
     @Override
     public String toString() {
-      final ServerSession session = context.session;
+      return taskName;
-      final String who = session.getUsername();
-      return getFullCommandLine() + " (" + who + ")";
     }
   }
 

gerrit-sshd/src/main/java/com/google/gerrit/sshd/CommandFactoryProvider.java

@@ -14,7 +14,7 @@
 
 package com.google.gerrit.sshd;
 
-import com.google.gerrit.sshd.SshScopes.Context;
+import com.google.gerrit.sshd.SshScope.Context;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 
@@ -59,7 +59,6 @@ class CommandFactoryProvider implements Provider<CommandFactory> {
     private OutputStream out;
     private OutputStream err;
     private ExitCallback exit;
-    private ServerSession session;
     private Context ctx;
     private DispatchCommand cmd;
     private boolean logged;
@@ -85,16 +84,13 @@ class CommandFactoryProvider implements Provider<CommandFactory> {
     }
 
     public void setSession(final ServerSession session) {
-      this.session = session;
+      this.ctx = new Context(session.getAttribute(SshSession.KEY));
     }
 
     public void start(final Environment env) throws IOException {
       synchronized (this) {
-        final Context old = SshScopes.current.get();
+        final Context old = SshScope.set(ctx);
         try {
-          ctx = new Context(session);
-          SshScopes.current.set(ctx);
-
           cmd = dispatcher.get();
           cmd.setCommandLine(commandLine);
           cmd.setInputStream(in);
@@ -115,7 +111,7 @@ class CommandFactoryProvider implements Provider<CommandFactory> {
           });
           cmd.start(env);
         } finally {
-          SshScopes.current.set(old);
+          SshScope.set(old);
         }
       }
     }
@@ -150,15 +146,14 @@ class CommandFactoryProvider implements Provider<CommandFactory> {
     public void destroy() {
       synchronized (this) {
         if (cmd != null) {
-          final Context old = SshScopes.current.get();
+          final Context old = SshScope.set(ctx);
           try {
-            SshScopes.current.set(ctx);
             cmd.destroy();
             log(BaseCommand.STATUS_CANCEL);
           } finally {
             ctx = null;
             cmd = null;
-            SshScopes.current.set(old);
+            SshScope.set(old);
           }
         }
       }

gerrit-sshd/src/main/java/com/google/gerrit/sshd/DatabasePasswordAuth.java

@@ -14,15 +14,14 @@
 
 package com.google.gerrit.sshd;
 
-import static com.google.gerrit.sshd.SshUtil.AUTH_ATTEMPTED_AS;
-import static com.google.gerrit.sshd.SshUtil.AUTH_ERROR;
-import static com.google.gerrit.sshd.SshUtil.CURRENT_ACCOUNT;
-
 import com.google.gerrit.reviewdb.AccountExternalId;
+import com.google.gerrit.server.AccessPath;
+import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.account.AccountCache;
 import com.google.gerrit.server.account.AccountState;
-import com.google.gerrit.sshd.SshScopes.Context;
+import com.google.gerrit.sshd.SshScope.Context;
 import com.google.inject.Inject;
+import com.google.inject.Provider;
 import com.google.inject.Singleton;
 
 import org.apache.mina.core.future.IoFuture;
@@ -30,6 +29,8 @@ import org.apache.mina.core.future.IoFutureListener;
 import org.apache.sshd.server.PasswordAuthenticator;
 import org.apache.sshd.server.session.ServerSession;
 
+import java.net.SocketAddress;
+
 /**
  * Authenticates by password through {@link AccountExternalId} entities.
  */
@@ -37,54 +38,63 @@ import org.apache.sshd.server.session.ServerSession;
 class DatabasePasswordAuth implements PasswordAuthenticator {
   private final AccountCache accountCache;
   private final SshLog log;
+  private final IdentifiedUser.GenericFactory userFactory;
 
   @Inject
-  DatabasePasswordAuth(final AccountCache ac, final SshLog l) {
+  DatabasePasswordAuth(final AccountCache ac, final SshLog l,
+      final IdentifiedUser.GenericFactory uf) {
     accountCache = ac;
     log = l;
+    userFactory = uf;
   }
 
   @Override
   public boolean authenticate(final String username, final String password,
       final ServerSession session) {
+    final SshSession sd = session.getAttribute(SshSession.KEY);
+
     AccountState state = accountCache.getByUsername(username);
     if (state == null) {
-      return fail(username, session, "user-not-found");
+      sd.authenticationError(username, "user-not-found");
+      return false;
     }
 
     final String p = state.getPassword(username);
     if (p == null) {
-      return fail(username, session, "no-password");
+      sd.authenticationError(username, "no-password");
+      return false;
     }
 
     if (!p.equals(password)) {
-      return fail(username, session, "incorrect-password");
+      sd.authenticationError(username, "incorrect-password");
+      return false;
     }
 
-    if (session.setAttribute(CURRENT_ACCOUNT, state.getAccount().getId()) == null) {
+    if (sd.getCurrentUser() == null) {
+      sd.authenticationSuccess(username, createUser(sd, state));
+
       // If this is the first time we've authenticated this
       // session, record a login event in the log and add
       // a close listener to record a logout event.
       //
-      final Context ctx = new Context(session);
+      Context ctx = new Context(sd);
-      final Context old = SshScopes.current.get();
+      Context old = SshScope.set(ctx);
       try {
-        SshScopes.current.set(ctx);
         log.onLogin();
       } finally {
-        SshScopes.current.set(old);
+        SshScope.set(old);
       }
 
       session.getIoSession().getCloseFuture().addListener(
           new IoFutureListener<IoFuture>() {
             @Override
             public void operationComplete(IoFuture future) {
-              final Context old = SshScopes.current.get();
+              final Context ctx = new Context(sd);
+              final Context old = SshScope.set(ctx);
               try {
-                SshScopes.current.set(ctx);
                 log.onLogout();
               } finally {
-                SshScopes.current.set(old);
+                SshScope.set(old);
               }
             }
           });
@@ -93,10 +103,13 @@ class DatabasePasswordAuth implements PasswordAuthenticator {
     return true;
   }
 
-  private static boolean fail(final String username,
+  private IdentifiedUser createUser(final SshSession sd,
-      final ServerSession session, final String err) {
+      final AccountState state) {
-    session.setAttribute(AUTH_ATTEMPTED_AS, username);
+    return userFactory.create(AccessPath.SSH, new Provider<SocketAddress>() {
-    session.setAttribute(AUTH_ERROR, err);
+      @Override
-    return false;
+      public SocketAddress get() {
+        return sd.getRemoteAddress();
+      }
+    }, state.getAccount().getId());
   }
 }

gerrit-sshd/src/main/java/com/google/gerrit/sshd/DatabasePubKeyAuth.java

@@ -14,13 +14,12 @@
 
 package com.google.gerrit.sshd;
 
-import static com.google.gerrit.sshd.SshUtil.AUTH_ATTEMPTED_AS;
-import static com.google.gerrit.sshd.SshUtil.AUTH_ERROR;
-import static com.google.gerrit.sshd.SshUtil.CURRENT_ACCOUNT;
-
 import com.google.gerrit.reviewdb.AccountSshKey;
-import com.google.gerrit.sshd.SshScopes.Context;
+import com.google.gerrit.server.AccessPath;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.sshd.SshScope.Context;
 import com.google.inject.Inject;
+import com.google.inject.Provider;
 import com.google.inject.Singleton;
 
 import org.apache.mina.core.future.IoFuture;
@@ -28,6 +27,7 @@ import org.apache.mina.core.future.IoFutureListener;
 import org.apache.sshd.server.PublickeyAuthenticator;
 import org.apache.sshd.server.session.ServerSession;
 
+import java.net.SocketAddress;
 import java.security.PublicKey;
 
 /**
@@ -37,15 +37,20 @@ import java.security.PublicKey;
 class DatabasePubKeyAuth implements PublickeyAuthenticator {
   private final SshKeyCacheImpl sshKeyCache;
   private final SshLog log;
+  private final IdentifiedUser.GenericFactory userFactory;
 
   @Inject
-  DatabasePubKeyAuth(final SshKeyCacheImpl skc, final SshLog l) {
+  DatabasePubKeyAuth(final SshKeyCacheImpl skc, final SshLog l,
+      final IdentifiedUser.GenericFactory uf) {
     sshKeyCache = skc;
     log = l;
+    userFactory = uf;
   }
 
   public boolean authenticate(final String username,
       final PublicKey suppliedKey, final ServerSession session) {
+    final SshSession sd = session.getAttribute(SshSession.KEY);
+
     final Iterable<SshKeyCacheEntry> keyList = sshKeyCache.get(username);
     final SshKeyCacheEntry key = find(keyList, suppliedKey);
     if (key == null) {
@@ -57,7 +62,8 @@ class DatabasePubKeyAuth implements PublickeyAuthenticator {
       } else {
         err = "no-matching-key";
       }
-      return fail(username, session, err);
+      sd.authenticationError(username, err);
+      return false;
     }
 
     // Double check that all of the keys are for the same user account.
@@ -68,34 +74,36 @@ class DatabasePubKeyAuth implements PublickeyAuthenticator {
     //
     for (final SshKeyCacheEntry otherKey : keyList) {
       if (!key.getAccount().equals(otherKey.getAccount())) {
-        return fail(username, session, "keys-cross-accounts");
+        sd.authenticationError(username, "keys-cross-accounts");
+        return false;
       }
     }
 
-    if (session.setAttribute(CURRENT_ACCOUNT, key.getAccount()) == null) {
+    if (sd.getCurrentUser() == null) {
+      sd.authenticationSuccess(username, createUser(sd, key));
+
       // If this is the first time we've authenticated this
       // session, record a login event in the log and add
       // a close listener to record a logout event.
       //
-      final Context ctx = new Context(session);
+      Context ctx = new Context(sd);
-      final Context old = SshScopes.current.get();
+      Context old = SshScope.set(ctx);
       try {
-        SshScopes.current.set(ctx);
         log.onLogin();
       } finally {
-        SshScopes.current.set(old);
+        SshScope.set(old);
       }
 
       session.getIoSession().getCloseFuture().addListener(
           new IoFutureListener<IoFuture>() {
             @Override
             public void operationComplete(IoFuture future) {
-              final Context old = SshScopes.current.get();
+              final Context ctx = new Context(sd);
+              final Context old = SshScope.set(ctx);
               try {
-                SshScopes.current.set(ctx);
                 log.onLogout();
               } finally {
-                SshScopes.current.set(old);
+                SshScope.set(old);
               }
             }
           });
@@ -104,11 +112,14 @@ class DatabasePubKeyAuth implements PublickeyAuthenticator {
     return true;
   }
 
-  private static boolean fail(final String username,
+  private IdentifiedUser createUser(final SshSession sd,
-      final ServerSession session, final String err) {
+      final SshKeyCacheEntry key) {
-    session.setAttribute(AUTH_ATTEMPTED_AS, username);
+    return userFactory.create(AccessPath.SSH, new Provider<SocketAddress>() {
-    session.setAttribute(AUTH_ERROR, err);
+      @Override
-    return false;
+      public SocketAddress get() {
+        return sd.getRemoteAddress();
+      }
+    }, key.getAccount());
   }
 
   private SshKeyCacheEntry find(final Iterable<SshKeyCacheEntry> keyList,

gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshCurrentUserProvider.java

@@ -1,4 +1,4 @@
-// Copyright (C) 2009 The Android Open Source Project
+// Copyright (C) 2010 The Android Open Source Project
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -14,31 +14,22 @@
 
 package com.google.gerrit.sshd;
 
-import com.google.gerrit.reviewdb.Account;
+import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.AccessPath;
-import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.sshd.SshScopes.Context;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
-import com.google.inject.ProvisionException;
 import com.google.inject.Singleton;
 
 @Singleton
-class SshCurrentUserProvider implements Provider<IdentifiedUser> {
+class SshCurrentUserProvider implements Provider<CurrentUser> {
-  private final IdentifiedUser.RequestFactory factory;
+  private final Provider<SshSession> session;
 
   @Inject
-  SshCurrentUserProvider(final IdentifiedUser.RequestFactory f) {
+  SshCurrentUserProvider(final Provider<SshSession> s) {
-    factory = f;
+    session = s;
   }
 
   @Override
-  public IdentifiedUser get() {
+  public CurrentUser get() {
-    final Context ctx = SshScopes.getContext();
+    return session.get().getCurrentUser();
-    final Account.Id id = ctx.session.getAttribute(SshUtil.CURRENT_ACCOUNT);
-    if (id == null) {
-      throw new ProvisionException("User not yet authenticated");
-    }
-    return factory.create(AccessPath.SSH, id);
   }
 }

gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshDaemon.java

@@ -20,7 +20,6 @@ import com.google.gerrit.server.ssh.SshInfo;
 import com.google.gerrit.server.util.IdGenerator;
 import com.google.gerrit.server.util.SocketUtil;
 import com.google.inject.Inject;
-import com.google.inject.Key;
 import com.google.inject.Singleton;
 
 import com.jcraft.jsch.HostKey;
@@ -85,7 +84,6 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
@@ -157,20 +155,18 @@ public class SshDaemon extends SshServer implements SshInfo, LifecycleListener {
         }
 
         final ServerSession s = (ServerSession) super.createSession(io);
-        s.setAttribute(SshUtil.REMOTE_PEER, io.getRemoteAddress());
+        final int id = idGenerator.next();
-        s.setAttribute(SshUtil.SESSION_ID, idGenerator.next());
+        final SocketAddress peer = io.getRemoteAddress();
-        s.setAttribute(SshScopes.sessionMap, new HashMap<Key<?>, Object>());
+        final SshSession sd = new SshSession(id, peer);
+        s.setAttribute(SshSession.KEY, sd);
 
         // Log a session close without authentication as a failure.
         //
         io.getCloseFuture().addListener(new IoFutureListener<IoFuture>() {
           @Override
           public void operationComplete(IoFuture future) {
-            if (s.getUsername() == null /* not authenticated */) {
+            if (sd.isAuthenticationError()) {
-              String username = s.getAttribute(SshUtil.AUTH_ATTEMPTED_AS);
+              sshLog.onAuthFail(sd);
-              if (username != null) {
-                sshLog.onAuthFail(s, username);
-              }
             }
           }
         });

gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshIdentifiedUserProvider.java

@@ -0,0 +1,43 @@
+// Copyright (C) 2010 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.sshd;
+
+import com.google.gerrit.common.errors.NotSignedInException;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.ProvisionException;
+import com.google.inject.Singleton;
+
+@Singleton
+class SshIdentifiedUserProvider implements Provider<IdentifiedUser> {
+  private final Provider<SshSession> session;
+
+  @Inject
+  SshIdentifiedUserProvider(final Provider<SshSession> s) {
+    session = s;
+  }
+
+  @Override
+  public IdentifiedUser get() {
+    final CurrentUser user = session.get().getCurrentUser();
+    if (user instanceof IdentifiedUser) {
+      return (IdentifiedUser) user;
+    }
+    throw new ProvisionException(NotSignedInException.MESSAGE,
+        new NotSignedInException());
+  }
+}

gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshLog.java

@@ -15,10 +15,11 @@
 package com.google.gerrit.sshd;
 
 import com.google.gerrit.lifecycle.LifecycleListener;
+import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.config.SitePaths;
 import com.google.gerrit.server.util.IdGenerator;
-import com.google.gerrit.sshd.SshScopes.Context;
+import com.google.gerrit.sshd.SshScope.Context;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
@@ -31,14 +32,10 @@ import org.apache.log4j.Level;
 import org.apache.log4j.Logger;
 import org.apache.log4j.spi.ErrorHandler;
 import org.apache.log4j.spi.LoggingEvent;
-import org.apache.sshd.server.session.ServerSession;
 import org.eclipse.jgit.util.QuotedString;
 
 import java.io.File;
 import java.io.IOException;
-import java.net.InetAddress;
-import java.net.InetSocketAddress;
-import java.net.SocketAddress;
 import java.text.SimpleDateFormat;
 import java.util.Calendar;
 import java.util.Date;
@@ -55,15 +52,12 @@ class SshLog implements LifecycleListener {
   private static final String P_EXEC = "executionTime";
   private static final String P_STATUS = "status";
 
-  private final Provider<ServerSession> session;
+  private final Provider<SshSession> session;
-  private final Provider<IdentifiedUser> user;
   private final AsyncAppender async;
 
   @Inject
-  SshLog(final Provider<ServerSession> session,
+  SshLog(final Provider<SshSession> session, final SitePaths site) {
-      final Provider<IdentifiedUser> user, final SitePaths site) {
     this.session = session;
-    this.user = user;
 
     final DailyRollingFileAppender dst = new DailyRollingFileAppender();
     dst.setName(LOG_NAME);
@@ -95,20 +89,16 @@ class SshLog implements LifecycleListener {
   }
 
   void onLogin() {
-    final ServerSession s = session.get();
+    async.append(log("LOGIN FROM " + session.get().getRemoteAddressAsString()));
-    final SocketAddress addr = s.getIoSession().getRemoteAddress();
-    async.append(log("LOGIN FROM " + format(addr)));
   }
 
-  void onAuthFail(final ServerSession s, final String username) {
+  void onAuthFail(final SshSession sd) {
-    final SocketAddress addr = s.getIoSession().getRemoteAddress();
-
     final LoggingEvent event = new LoggingEvent( //
         Logger.class.getName(), // fqnOfCategoryClass
         null, // logger (optional)
         System.currentTimeMillis(), // when
         Level.INFO, // level
-        "AUTH FAILURE FROM " + format(addr), // message text
+        "AUTH FAILURE FROM " + sd.getRemoteAddressAsString(), // message text
         "SSHD", // thread name
         null, // exception information
         null, // current NDC string
@@ -116,10 +106,10 @@ class SshLog implements LifecycleListener {
         null // MDC properties
         );
 
-    event.setProperty(P_SESSION, id(s.getAttribute(SshUtil.SESSION_ID)));
+    event.setProperty(P_SESSION, id(sd.getSessionId()));
-    event.setProperty(P_USER_NAME, username);
+    event.setProperty(P_USER_NAME, sd.getUsername());
 
-    final String error = s.getAttribute(SshUtil.AUTH_ERROR);
+    final String error = sd.getAuthenticationError();
     if (error != null) {
       event.setProperty(P_STATUS, error);
     }
@@ -165,8 +155,8 @@ class SshLog implements LifecycleListener {
   }
 
   private LoggingEvent log(final String msg) {
-    final ServerSession s = session.get();
+    final SshSession sd = session.get();
-    final IdentifiedUser u = user.get();
+    final CurrentUser user = sd.getCurrentUser();
 
     final LoggingEvent event = new LoggingEvent( //
         Logger.class.getName(), // fqnOfCategoryClass
@@ -181,32 +171,24 @@ class SshLog implements LifecycleListener {
         null // MDC properties
         );
 
-    event.setProperty(P_SESSION, id(s.getAttribute(SshUtil.SESSION_ID)));
+    event.setProperty(P_SESSION, id(sd.getSessionId()));
-    event.setProperty(P_USER_NAME, u.getUserName());
+
-    event.setProperty(P_ACCOUNT_ID, "a/" + u.getAccountId().toString());
+    String userName = "-", accountId = "-";
+
+    if (user instanceof IdentifiedUser) {
+      IdentifiedUser u = (IdentifiedUser) user;
+      userName = u.getAccount().getUserName();
+      accountId = "a/" + u.getAccountId().toString();
+    }
+
+    event.setProperty(P_USER_NAME, userName);
+    event.setProperty(P_ACCOUNT_ID, accountId);
 
     return event;
   }
 
-  private static String format(final SocketAddress remote) {
+  private static String id(final int id) {
-    if (remote instanceof InetSocketAddress) {
+    return IdGenerator.format(id);
-      final InetSocketAddress sa = (InetSocketAddress) remote;
-
-      final InetAddress in = sa.getAddress();
-      if (in != null) {
-        return in.getHostAddress();
-      }
-
-      final String hostName = sa.getHostName();
-      if (hostName != null) {
-        return hostName;
-      }
-    }
-    return remote.toString();
-  }
-
-  private static String id(final Integer id) {
-    return id != null ? IdGenerator.format(id) : "";
   }
 
   private static File resolve(final File logs_dir) {

gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshModule.java

@@ -38,18 +38,14 @@ import com.google.gerrit.util.cli.CmdLineParser;
 import com.google.gerrit.util.cli.OptionHandlerFactory;
 import com.google.gerrit.util.cli.OptionHandlerUtil;
 import com.google.inject.Key;
-import com.google.inject.Provider;
 import com.google.inject.TypeLiteral;
 import com.google.inject.assistedinject.FactoryProvider;
 import com.google.inject.servlet.RequestScoped;
-import com.google.inject.servlet.SessionScoped;
 
 import org.apache.sshd.common.KeyPairProvider;
-import org.apache.sshd.common.session.AbstractSession;
 import org.apache.sshd.server.CommandFactory;
 import org.apache.sshd.server.PasswordAuthenticator;
 import org.apache.sshd.server.PublickeyAuthenticator;
-import org.apache.sshd.server.session.ServerSession;
 import org.kohsuke.args4j.spi.OptionHandler;
 
 import java.net.SocketAddress;
@@ -60,10 +56,8 @@ public class SshModule extends FactoryModule {
 
   @Override
   protected void configure() {
-    bindScope(SessionScoped.class, SshScopes.SESSION);
+    bindScope(RequestScoped.class, SshScope.REQUEST);
-    bindScope(RequestScoped.class, SshScopes.REQUEST);
 
-    configureSessionScope();
     configureRequestScope();
     configureCmdLineParser();
 
@@ -96,30 +90,20 @@ public class SshModule extends FactoryModule {
     });
   }
 
-  private void configureSessionScope() {
-    bind(ServerSession.class).toProvider(new Provider<ServerSession>() {
-      @Override
-      public ServerSession get() {
-        return SshScopes.getContext().session;
-      }
-    }).in(SshScopes.SESSION);
-    bind(AbstractSession.class).to(ServerSession.class).in(SshScopes.SESSION);
-
-    bind(SocketAddress.class).annotatedWith(RemotePeer.class).toProvider(
-        new Provider<SocketAddress>() {
-          @Override
-          public SocketAddress get() {
-            return SshScopes.getContext().session
-                .getAttribute(SshUtil.REMOTE_PEER);
-          }
-        }).in(SshScopes.SESSION);
-  }
-
   private void configureRequestScope() {
+    bind(SshScope.Context.class).toProvider(SshScope.ContextProvider.class);
+
+    bind(SshSession.class).toProvider(SshScope.SshSessionProvider.class).in(
+        SshScope.REQUEST);
+    bind(SocketAddress.class).annotatedWith(RemotePeer.class).toProvider(
+        SshRemotePeerProvider.class).in(SshScope.REQUEST);
+
+    bind(CurrentUser.class).toProvider(SshCurrentUserProvider.class).in(
+        SshScope.REQUEST);
+    bind(IdentifiedUser.class).toProvider(SshIdentifiedUserProvider.class).in(
+        SshScope.REQUEST);
+
     install(new GerritRequestModule());
-    bind(IdentifiedUser.class).toProvider(SshCurrentUserProvider.class).in(
-        SshScopes.REQUEST);
-    bind(CurrentUser.class).to(IdentifiedUser.class);
   }
 
   private void configureCmdLineParser() {

gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshRemotePeerProvider.java

@@ -0,0 +1,36 @@
+// Copyright (C) 2010 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.sshd;
+
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+
+import java.net.SocketAddress;
+
+@Singleton
+class SshRemotePeerProvider implements Provider<SocketAddress> {
+  private final Provider<SshSession> session;
+
+  @Inject
+  SshRemotePeerProvider(final Provider<SshSession> s) {
+    session = s;
+  }
+
+  @Override
+  public SocketAddress get() {
+    return session.get().getRemoteAddress();
+  }
+}

gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshScope.java

@@ -19,51 +19,27 @@ import com.google.inject.OutOfScopeException;
 import com.google.inject.Provider;
 import com.google.inject.Scope;
 
-import org.apache.sshd.common.Session.AttributeKey;
-import org.apache.sshd.server.session.ServerSession;
-
 import java.util.HashMap;
 import java.util.Map;
 
 /** Guice scopes for state during an SSH connection. */
-class SshScopes {
+class SshScope {
   static class Context {
-    final ServerSession session;
+    private final SshSession session;
-    final Map<Key<?>, Object> map;
+    private final Map<Key<?>, Object> map;
+
     final long created;
     volatile long started;
     volatile long finished;
 
-    Context(final ServerSession s) {
+    Context(final SshSession s) {
       session = s;
       map = new HashMap<Key<?>, Object>();
       created = System.currentTimeMillis();
       started = created;
     }
-  }
 
-  static final AttributeKey<Map<Key<?>, Object>> sessionMap =
+    synchronized <T> T get(Key<T> key, Provider<T> creator) {
-      new AttributeKey<Map<Key<?>, Object>>();
-
-  static final ThreadLocal<Context> current = new ThreadLocal<Context>();
-
-  static Context getContext() {
-    final Context ctx = current.get();
-    if (ctx == null) {
-      throw new OutOfScopeException(
-          "Cannot access scoped object; not in request/command.");
-    }
-    return ctx;
-  }
-
-  /** Returns exactly one instance for the scope of the SSH connection. */
-  static final Scope SESSION = new Scope() {
-    public <T> Provider<T> scope(final Key<T> key, final Provider<T> creator) {
-      return new Provider<T>() {
-        public T get() {
-          final Context ctx = getContext();
-          final Map<Key<?>, Object> map = ctx.session.getAttribute(sessionMap);
-          synchronized (map) {
       @SuppressWarnings("unchecked")
       T t = (T) map.get(key);
       if (t == null) {
@@ -74,34 +50,43 @@ class SshScopes {
     }
   }
 
+  static class ContextProvider implements Provider<Context> {
     @Override
-        public String toString() {
+    public Context get() {
-          return String.format("%s[%s]", creator, SESSION);
+      return getContext();
     }
-      };
   }
 
+  static class SshSessionProvider implements Provider<SshSession> {
     @Override
-    public String toString() {
+    public SshSession get() {
-      return "SshScopes.SESSION";
+      return getContext().session;
+    }
+  }
+
+  private static final ThreadLocal<Context> current =
+      new ThreadLocal<Context>();
+
+  private static Context getContext() {
+    final Context ctx = current.get();
+    if (ctx == null) {
+      throw new OutOfScopeException("Not in command/request");
+    }
+    return ctx;
+  }
+
+  static Context set(Context ctx) {
+    Context old = current.get();
+    current.set(ctx);
+    return old;
   }
-  };
 
   /** Returns exactly one instance per command executed. */
   static final Scope REQUEST = new Scope() {
     public <T> Provider<T> scope(final Key<T> key, final Provider<T> creator) {
       return new Provider<T>() {
         public T get() {
-          final Map<Key<?>, Object> map = getContext().map;
+          return getContext().get(key, creator);
-          synchronized (map) {
-            @SuppressWarnings("unchecked")
-            T t = (T) map.get(key);
-            if (t == null) {
-              t = creator.get();
-              map.put(key, t);
-            }
-            return t;
-          }
         }
 
         @Override
@@ -117,6 +102,6 @@ class SshScopes {
     }
   };
 
-  private SshScopes() {
+  private SshScope() {
   }
 }

gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshSession.java

@@ -0,0 +1,104 @@
+// Copyright (C) 2010 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.sshd;
+
+import com.google.gerrit.server.CurrentUser;
+
+import org.apache.sshd.common.Session.AttributeKey;
+
+import java.net.InetAddress;
+import java.net.InetSocketAddress;
+import java.net.SocketAddress;
+
+/** Global data related to an active SSH connection. */
+public class SshSession {
+  /** ServerSession attribute key for this object instance. */
+  public static final AttributeKey<SshSession> KEY =
+      new AttributeKey<SshSession>();
+
+  private final int sessionId;
+  private final SocketAddress remoteAddress;
+  private final String remoteAsString;
+
+  private volatile CurrentUser identity;
+  private volatile String username;
+  private volatile String authError;
+
+  SshSession(final int sessionId, SocketAddress peer) {
+    this.sessionId = sessionId;
+    this.remoteAddress = peer;
+    this.remoteAsString = format(remoteAddress);
+  }
+
+  /** Unique session number, assigned during connect. */
+  public int getSessionId() {
+    return sessionId;
+  }
+
+  /** Identity of the authenticated user account on the socket. */
+  public CurrentUser getCurrentUser() {
+    return identity;
+  }
+
+  String getUsername() {
+    return username;
+  }
+
+  String getAuthenticationError() {
+    return authError;
+  }
+
+  void authenticationSuccess(String user, CurrentUser id) {
+    username = user;
+    identity = id;
+    authError = null;
+  }
+
+  void authenticationError(String user, String error) {
+    username = user;
+    identity = null;
+    authError = error;
+  }
+
+  /** @return {@code true} if the authentication did not succeed. */
+  boolean isAuthenticationError() {
+    return authError != null;
+  }
+
+  SocketAddress getRemoteAddress() {
+    return remoteAddress;
+  }
+
+  String getRemoteAddressAsString() {
+    return remoteAsString;
+  }
+
+  private static String format(final SocketAddress remote) {
+    if (remote instanceof InetSocketAddress) {
+      final InetSocketAddress sa = (InetSocketAddress) remote;
+
+      final InetAddress in = sa.getAddress();
+      if (in != null) {
+        return in.getHostAddress();
+      }
+
+      final String hostName = sa.getHostName();
+      if (hostName != null) {
+        return hostName;
+      }
+    }
+    return remote.toString();
+  }
+}

gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshUtil.java

@@ -14,20 +14,17 @@
 
 package com.google.gerrit.sshd;
 
-import com.google.gerrit.reviewdb.Account;
 import com.google.gerrit.reviewdb.AccountSshKey;
 
 import org.apache.commons.codec.binary.Base64;
 import org.apache.sshd.common.KeyPairProvider;
 import org.apache.sshd.common.SshException;
-import org.apache.sshd.common.Session.AttributeKey;
 import org.apache.sshd.common.util.Buffer;
 import org.eclipse.jgit.lib.Constants;
 
 import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.StringReader;
-import java.net.SocketAddress;
 import java.security.NoSuchAlgorithmException;
 import java.security.NoSuchProviderException;
 import java.security.PublicKey;
@@ -37,25 +34,6 @@ import java.security.spec.InvalidKeySpecException;
 
 /** Utilities to support SSH operations. */
 public class SshUtil {
-  /** Server session attribute holding the {@link Account.Id}. */
-  public static final AttributeKey<Account.Id> CURRENT_ACCOUNT =
-      new AttributeKey<Account.Id>();
-
-  /** Server session attribute holding the remote {@link SocketAddress}. */
-  public static final AttributeKey<SocketAddress> REMOTE_PEER =
-      new AttributeKey<SocketAddress>();
-
-  /** Server session attribute holding a unique session id. */
-  public static final AttributeKey<Integer> SESSION_ID =
-      new AttributeKey<Integer>();
-
-  /** Username the last authentication tried to perform as. */
-  static final AttributeKey<String> AUTH_ATTEMPTED_AS =
-      new AttributeKey<String>();
-
-  /** Error message from last authentication attempt. */
-  static final AttributeKey<String> AUTH_ERROR = new AttributeKey<String>();
-
   /**
    * Parse a public key into its Java type.
    *

gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/AdminShowConnections.java

@@ -14,12 +14,13 @@
 
 package com.google.gerrit.sshd.commands;
 
-import com.google.gerrit.reviewdb.Account;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.util.IdGenerator;
 import com.google.gerrit.sshd.AdminCommand;
 import com.google.gerrit.sshd.BaseCommand;
 import com.google.gerrit.sshd.SshDaemon;
-import com.google.gerrit.sshd.SshUtil;
+import com.google.gerrit.sshd.SshSession;
 import com.google.inject.Inject;
 
 import org.apache.mina.core.service.IoAcceptor;
@@ -89,17 +90,17 @@ final class AdminShowConnections extends BaseCommand {
     p.print("--------------------------------------------------------------\n");
     for (final IoSession io : list) {
       ServerSession s = (ServerSession) ServerSession.getSession(io, true);
+      SshSession sd = s != null ? s.getAttribute(SshSession.KEY) : null;
 
       final SocketAddress remoteAddress = io.getRemoteAddress();
       final long start = io.getCreationTime();
       final long idle = now - io.getLastIoTime();
-      final Integer id = s != null ? s.getAttribute(SshUtil.SESSION_ID) : null;
 
       p.print(String.format("%8s %8s %8s  %-15.15s %.30s\n", //
-          id(id), //
+          id(sd), //
           time(now, start), //
           age(idle), //
-          username(s), //
+          username(sd), //
           hostname(remoteAddress)));
     }
     p.print("--\n");
@@ -107,8 +108,8 @@ final class AdminShowConnections extends BaseCommand {
     p.flush();
   }
 
-  private static String id(final Integer id) {
+  private static String id(final SshSession sd) {
-    return id != null ? IdGenerator.format(id) : "";
+    return sd != null ? IdGenerator.format(sd.getSessionId()) : "";
   }
 
   private static String time(final long now, final long time) {
@@ -131,15 +132,26 @@ final class AdminShowConnections extends BaseCommand {
     return String.format("%02d:%02d:%02d", hr, min, sec);
   }
 
-  private String username(final ServerSession s) {
+  private String username(final SshSession sd) {
-    if (s == null) {
+    if (sd == null) {
       return "";
-    } else if (numeric) {
+    }
-      final Account.Id id = s.getAttribute(SshUtil.CURRENT_ACCOUNT);
+
-      return id != null ? "a/" + id.toString() : "";
+    final CurrentUser user = sd.getCurrentUser();
+    if (user instanceof IdentifiedUser) {
+      IdentifiedUser u = (IdentifiedUser) user;
+
+      if (!numeric) {
+        String name = u.getAccount().getUserName();
+        if (name != null && !name.isEmpty()) {
+          return name;
+        }
+      }
+
+      return "a/" + u.getAccountId().toString();
+
     } else {
-      final String user = s.getUsername();
+      return "";
-      return user != null ? user : "";
     }
   }