arrikto / oidc-authservice

This is a fork/refactoring of the ajmyyra/ambassador-auth-oidc project
MIT License
87 stars 65 forks source link

Opening a page protected by AuthService in multiple tabs while logged out will produce an error during login #53

Open apyrgio opened 3 years ago

apyrgio commented 3 years ago

Is this a bug report or feature request?

Describe the bug

Currently, we rely on cookies to verify that the state parameter has originated from the user's browser, and has not been maliciously planted in the URL. This way, we protect the user from logging in accidentally with another user's account.

The problem with this approach is that if a user opens the page that is protected by the authservice in two or more tabs, the state cookie will be overridden, and the user won't be able to login from the first page.

A side-effect of this issue is that the page that the user intended to visit, which authservice would redirect them to after login, will be lost.

How to Reproduce Steps to reproduce the behavior:

  1. Deploy AuthService.
  2. Open a page in the browser that is guarded by the AuthService.
  3. Open a second one.
  4. Try to login from the first page.
  5. You will see the following error: CSRF check failed.This may happen if you opened the login form in more than 1 tabs. Please try to login again.

Expected behavior

The user should be able to login from either page. If the user is already logged in one page and they attempt to log in from another one, they should be redirected to the page they intended to visit.

edwardzjl commented 1 year ago

I think the simplest way is to check whether current user has already logged in when handling oidc-callback, and skip the authentication flow if the user is already logged in.

But the redirection will be lost anyway, we can only rely on afterLoginRedirectURL:

func (s *server) callback(w http.ResponseWriter, r *http.Request) {
    ...
    // check current session
    if session, err := s.store.Get(r, userSessionCookie); err != nil {
        logger.Errorf("Failed to get session: %v", err)
    } else if !session.IsNew {
        // session already exists, which means the user already logged in
        destination := ""
        if s.afterLoginRedirectURL != "" {
            // Redirect to a predefined url from config.
            afterLoginRedirectURL := mustParseURL(s.afterLoginRedirectURL)
            destination = afterLoginRedirectURL.String()
        }
        logger.WithField("redirectTo", destination).
        Info("User already logged in, redirecting.")
        http.Redirect(w, r, destination, http.StatusFound)
        return
    }
    ...
deepakdeore2004 commented 6 months ago

any chance to have progress on this?