Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sometimes logins fail with login in progress #1069

Closed
2 of 5 tasks
JordanMarr opened this issue Oct 23, 2019 · 24 comments
Closed
2 of 5 tasks

Sometimes logins fail with login in progress #1069

JordanMarr opened this issue Oct 23, 2019 · 24 comments
Assignees
Labels
bug A problem that needs to be fixed for the feature to function as intended.

Comments

@JordanMarr
Copy link

JordanMarr commented Oct 23, 2019

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[x] Bug report  
[ ] Performance issue
[ ] Feature request
[ ] Documentation issue or request
[ ] Other... Please describe:

Browser:

  • Chrome version XX
  • Firefox version XX
  • IE version XX
  • Edge version XX
  • Safari version XX

Library version


Library version: 1.2.0-beta.2

Current behavior

Sometimes I can login successfully, but other times it fails and it shows a login is "InProgress" in my Session Storage. Clearing cookies or clearing session storage won't fix it, but clearing both will. Opening in "Incognito" mode or "InPrivate" mode will also fix it.

Should I be detecting this weird state and trying to programmatically clear the session storage and cookies?

Expected behavior

It should either log in correctly every time, or it should automatically clear the session state and cookies, or whatever it needs to clear.

Minimal reproduction of the problem with instructions

I still don't understand what is causing my logins to sometimes fail with the "InProgress" status so it's hard to give steps to reproduce.

@JordanMarr
Copy link
Author

Manually clearing session seems to work; still need to do more testing...

function logRedirectCallbackErrors(error: MSAL.AuthError, response: MSAL.AuthResponse | undefined) {
        if (error) {
            console.error("Redirect Callback Error: " + error);
            setDeniedReason("Unable to login.")
            setIsAccessDenied(true);
            // Clear session to remove "login in process" error
            sessionStorage.clear();
        }
    }

@sameerag
Copy link
Member

@JordanMarr Is this happening only with session storage or is it seen for local storage too? Is it possible to share your sample code for authentication so we can try reproducing the issue at our end?

@sameerag sameerag added the bug A problem that needs to be fixed for the feature to function as intended. label Oct 28, 2019
@JordanMarr
Copy link
Author

@JordanMarr Is this happening only with session storage or is it seen for local storage too? Is it possible to share your sample code for authentication so we can try reproducing the issue at our end?

My authentication code (as a React hook) is below (my tenant id and authority replaced with placeholders).
I know it's not exactly like the examples, but this is the best I could do to get it working.

import React, { useState, useEffect } from 'react';
import API from './api';
import * as MSAL from 'msal'
import { navigate } from 'hookrouter';

export interface IUser {
    name: string,
    email: string,
    isPM: boolean,
    isAdmin: boolean
}

export const useAuth = () => {
    const msalConfig: MSAL.Configuration = {
        auth: {
            clientId: 'my tenant id',
            authority: 'my authority'
        },
        cache: {
            storeAuthStateInCookie: true
        }
    };

    const tokenReq: MSAL.AuthenticationParameters = {
        scopes: ["api://my tenant id/user_impersonation"]
    };

    const [isAuthenticated, setIsAuthenticated] = useState(false);
    const [isAccessDenied, setIsAccessDenied] = useState(false);
    const [deniedReason, setDeniedReason] = useState("");
    const [iuser, setIuser] = useState<IUser>({ name: "Not Authenticated", email: "-", isPM: false, isAdmin: false });

    const addBearerTokenToRequests = (token: string) => {
        API.interceptors.request.use(req => {
            req.headers['Authorization'] = `Bearer ${token}`;
            return req;
        });
    };

    const handle401Errors = () => {
        API.interceptors.response.use(resp => {
            return resp;
        }, error => {
            if (error.response.status === 401) {
                setDeniedReason(error.response.data);
                setIsAuthenticated(false);
                setIsAccessDenied(true);
                console.error("401 Error: User is not authorized to access this resource. " + error);
            }
            throw error;
        });
    };

    useEffect(() => {

        handle401Errors();
        authenticate();

    }, []);

    return { isAuthenticated, isAccessDenied, deniedReason, user: iuser, logout };

    function urlContains(str: string) {
        return window.location.href.indexOf(str) > -1;
    }

    async function authenticate() {
        let msal = new MSAL.UserAgentApplication(msalConfig);

        let acct = await msal.getAccount();

        if (!acct) { // Users need to login                                                
            try {
                console.log('Redirecting to login...');

                // A handler is required or loginRedirect won't work.
                msal.handleRedirectCallback(logRedirectCallbackErrors);
                navigate('/'); // User must sign in from the root url (must match the redirect url configured in the Azure App Registration)
                msal.loginRedirect(tokenReq);
            } catch (err3) {
                console.error(`Unable to authenticate: ${err3}`);
            }
        } else if (!urlContains('access_token')) { // user id exists - acquire token silently
            try {
                console.log('Acquiring a token silently...');
                let resp = await msal.acquireTokenSilent(tokenReq);
                if (resp.accessToken) {
                    console.log('Acquired an access token.');

                    setIuser({ ...iuser, name: resp.account.name, email: resp.account.userName });
                    addBearerTokenToRequests(resp.accessToken);

                    try {
                        // Get user with claims (isPM, isAdmin, etc)
                        const { data } = await API.get<IUser>('/api/auth/user');

                        // Update return values
                        setIsAuthenticated(true);
                        setIuser(data);
                    } catch (error) {
                        setDeniedReason(error.response.data);
                        setIsAuthenticated(false);
                        setIsAccessDenied(true);
                    }
                } else {
                    console.error('Unable to obtain an access token.');
                    setIsAccessDenied(true);
                }
            } catch (err) {
                try {
                    console.log('Redirecting to login...');

                    // A handler is required or loginRedirect won't work.
                    msal.handleRedirectCallback(logRedirectCallbackErrors);
                    navigate('/'); // User must sign in from the root url (must match the redirect url configured in the Azure App Registration)
                    msal.loginRedirect(tokenReq);
                } catch (err2) {
                    console.error(`Unable to aquire an access token: ${err2}`);
                    setIsAccessDenied(true);
                }
            }
        }
    }

    function logRedirectCallbackErrors(error: MSAL.AuthError, response: MSAL.AuthResponse | undefined) {
        if (error) {
            console.error("Redirect Callback Error: " + error);
            setDeniedReason("Unable to login.")
            setIsAccessDenied(true);
            // Clear session to remove "login in process" error
            sessionStorage.clear();
        }
    }

    async function logout() {
        let msal = new MSAL.UserAgentApplication(msalConfig);
        msal.logout();
    }
}

@AndrewCraswell
Copy link
Contributor

I ran into this today and found an easy way to repro.

  1. Clear any active sessions from the cache and delete cookies.
  2. Navigate to the site, but before initiating a login disconnect from your internet.
  3. Initiate login. You should encounter a login error since MSAL can't resolve the proper endpoints.
  4. Reconnect the internet, and try to login again. This will fail because "Login in progress".

The only way to successfully login after encountering this is as @JordanMarr said, delete sessionStorage and any cookies.

@sameerag
Copy link
Member

@AndrewCraswell Thanks for the steps. We are working on something similar to this (but for other error types) using #1024; the initial implementation has some flaws and I am working on fixing this. The PR is primarily to clean up intermittent cache which is created during login/token calls when the flow bails out in an error case.

What you brought up is an interesting edge case,msal js should be in this case able to identify a stale request since the NW is disconnected. Let me check if that is feasible and reach out.

@sameerag sameerag self-assigned this Oct 29, 2019
@AndrewCraswell
Copy link
Contributor

Did you mean #1042? I had seen that PR earlier but for some reason thought it had already been merged.

@sameerag
Copy link
Member

Sorry yes. It is #1042. It isn't merged as I think our approach causes issues in some error cases. I will do a new version soon. But as I stated, the use case you mentioned is a corner case and with concurrent requests, we need to identify a stale request vs on going one which is tricky as we do not remember application state. Will reach out once I have a solution in place.

@AndrewCraswell
Copy link
Contributor

AndrewCraswell commented Nov 11, 2019

I was able to find another way to repro this issue in a bit more egregious way. These steps will break when using Redirect login method:

  1. Navigate to the site, execute login.
  2. After being redirected to the login page, use the URL bar to navigate back to your page (same session).
  3. Execute login flow again. You will receive ClientAuthError with error code of login_progress_error.

This example was done with sessionStorage.

See also:
syncweek-react-aad/react-aad#160

@sameerag
Copy link
Member

Yes. The above example makes sense. Would you be able to test if I send a private build?

@AndrewCraswell
Copy link
Contributor

@sameerag Absolutely, would be happy to.

@AndrewCraswell
Copy link
Contributor

AndrewCraswell commented Nov 22, 2019

@sameerag Was this one of the items addressed in Beta 4? I'm assuming not since I can still repro it.

Weird question though, because we've hit this issue again in a new scenario with a corporate customer. On Edge browsers when the user initiates a login, the redirect to login.microsoftonline.com is prevented, and instead opens in a new instance of IE 11... Because of this when they complete the login flow in IE 11 the user receives login_in_progress which seems perfectly valid to me. This is a stretch, but have any of you encountered such an issue before? I'm guessing this company has some sort of group policy causing this. I'm still investigating whether this is something we can prevent on our side.

@sameerag
Copy link
Member

@AndrewCraswell beta-5 isn't released yet. We plan to do it tomorrow and release 1.2.0 around Dec 6-8th.
I did not come across the issue you mentioned on Edge browsers, I can check with my team and reach out.

@sameerag
Copy link
Member

sameerag commented Dec 4, 2019

#1042 is now merged to dev. beta release and a mainline release will follow soon.

@rajasekhar2
Copy link

@sameerag is this issue fixed in beta?

@aappddeevv
Copy link

aappddeevv commented Dec 13, 2019

I'm still having this issue in 1.2.0 with the redirect. I entered this state from a bad login and now it just loops. I'm using local storage. There are no cookies set when I encounter this and even if I clear local storage and make sure there are no cookies, I'm still stuck in the endless loop. Also, where is it documented that the redirect will return to the initiating page but with a #id_token stuck on my url?`

Per the comments above, I guess that once a cookie indicating login in progress gets set on the https://login.microsoftonline.com origin, it is never removed even if you signout and is passed to the login request from my app. Cleaning the login.microsoftonline.com cookie and origin storage for my app does fix the loop.

@sameerag
Copy link
Member

@aappddeevv The loop is probably triggered from AAD where if a given request after failing is retried multiple times. Since this issue is partially addressing the cache clean-up issues, it gets confusing to track a new use case with the same ticket.

Can you please:

  • Raise a new issue with clear steps of repro for this loop?
  • Please share the msal version and any other details (browser, config etc) that will be useful for debug

Same applies for anyone following this thread. Please raise a new github issue helping us track different use cases clearly. Closing this ticket.

@brayanL
Copy link

brayanL commented Feb 7, 2020

I have the same issue with B2C sometimes the app open a new login tab and AuthenticationState.InProgress keep in true. Sometimes the new tab is closed automatically after 3 or more minutes. @sameerag Should I open a new ticket?

@betterliyu
Copy link

betterliyu commented Mar 27, 2020

I still meet the similar issue using react v1.2.1. The page don't redirect to the home page. Just stay on that page with url domain/#id_token=XXX. And status is inProgress in sessionStorage.
image

async componentDidMount() {
    if (msalApp.urlContainsHash(window.location.hash) && isInIframe()) {
        return null;
    }
    msalApp.handleRedirectCallback(error => {
        if (error) {
            const errorMessage = error.errorMessage ? error.errorMessage : "Unable to acquire access token.";
            // setState works as long as navigateToLoginRequestUrl: false
            this.setState({
                error: errorMessage
            });
        }
    });

    const account = msalApp.getAccount();
    this.setState({
        account
    });

    if (!account && !msalApp.urlContainsHash(window.location.hash) && !msalApp.getLoginInProgress()) {
        return this.onSignIn(true);
    }

    if (account) {
        this.acquireTokens()
    }
}

@crobin1
Copy link

crobin1 commented Jul 2, 2020

Got the exact same problem as @betterliyu. Why is it even closed?

@SpicySyntax
Copy link

Same problem as @betterliyu and @crobin1

@crobin1
Copy link

crobin1 commented Jul 17, 2020

I've noticed if you are already logged-in with your Azure AD account somewhere and then browse the web app secured by msal.js in the same browser session or another tab, it's stuck in "In Progress". But if you open another window of the same browser and try again, it works. Closing completely the browser and reopening it work as well.

@crobin1
Copy link

crobin1 commented Jul 17, 2020

@sameerag Could you reconsider this issue and reopen it?

@tnorling
Copy link
Collaborator

@crobin1 Please open a new ticket and our livesite engineer would be happy to help you solve this. Thanks!

@jaslloyd
Copy link

@crobin1 @tnorling Created one here: #2071

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug A problem that needs to be fixed for the feature to function as intended.
Projects
None yet
Development

No branches or pull requests