[Bug?]: Azure Active Directory Authentication – Redirect to authorization endpoint despite @skipAuth

[m3t]

What's not working?

@jeliasson @Tobbe

When Azure Active Directory Authentication is integrated as AuthProvider, I get a redirect right away to the authorization endpoint only by using a public query.

// api/src/graphql/foobars.sdl.ts

export const schema = gql`
  type Foobar {
    epoch: BigInt!
  }

  type Query {
    foobar: Foobar @skipAuth
  }
`

// api/src/services/foobars/foobars.ts

import type { QueryResolvers } from 'types/graphql'

export const foobar: QueryResolvers['foobar'] = () => ({ epoch: Date.now() })

So far, this is my understanding for the login process:

• https://azuread.github.io/microsoft-authentication-library-for-js/ref/classes/_azure_msal_browser.publicclientapplication.html#loginredirect
• https://azuread.github.io/microsoft-authentication-library-for-js/ref/modules/_azure_msal_browser.html#redirectrequest

How do we reproduce the bug?

Version 3.6.1

• https://github.com/m3t/redwood-demo/tree/azure-auth-public-query (yarn create)
• https://github.com/m3t/redwood-gitpod-starter/tree/azure-auth-public-query (fork, recommended here)

What's your environment? (If it applies)

Node: 16.14.2
Yarn: 3.2.3
@redwoodjs/core: 3.6.1

Are you interested in working on this?

• I'm interested in working on this

[jeliasson]

@m3t Can you reproduce this in a fresh 3.6.1 installation?

[m3t]

@jeliasson
Indeed! Please have a look in https://github.com/m3t/redwood-gitpod-starter/tree/azure-auth-public-query

[jeliasson]

@m3t I saw that earlier, and it has references from 0.45.0. Have you tried from a fresh out-of-the-box installation of 3.6.1?

[m3t]

@jeliasson
No, I did not use yarn create, instead I followed this recommendation:
https://github.com/redwoodjs/redwood/blob/v3.6.1/CONTRIBUTING.md#option-2-fork-the-starter-repo

[m3t]

@jeliasson
I will give it a try with yarn create again and keep you informed.
Thank you for your advice

[m3t]

@jeliasson
Unfortunately I can reproduce the same unwanted behaviour in the following repo:
https://github.com/m3t/redwood-demo/tree/azure-auth-public-query

I've installed it with following versions and commands :

# Node: 16.14.2
# Yarn: 3.2.3
# @redwoodjs/core: 3.6.1

yarn create redwood-app --ts ./redwood-demo

yarn rw setup auth azureActiveDirectory

[jeliasson]

@m3t Thanks for that! By any chance you can share a temporary Azure App registration and it's environment variables that could be used for troubleshooting? Ideally, the redirect URIs would include http://localhost:8910 😊

@Tobbe Without diving too deep into the issue at hand, do you know why a anonymous request to GraphQL with the skipAuth directive would be ignored and treated as if it was a protected endpoint?

[m3t]

@jeliasson
You've been invited to a private repo with the data you've asked for.

[tilmann]

We are experiencing in the same problem. Did anyone find a solution or a workaround?

[tilmann]

Short update: I was able to reproduce the behavior with

"@redwoodjs/auth-azure-active-directory-setup": "4.0.0-rc.437",
"@redwoodjs/core": "^4.0.0-canary.451",

(Thanks for all the work you did there @Tobbe.)

[rafaelhutter]

Hi @jeliasson and @Tobbe ,
thanks a lot for the great work you did with this framework. We are currently a bit stuck in our project because of this issue. Are there any news on this?
Thanks a lot!

[m3t]

@jeliasson

Have you tried out the login data I gave you?
Any progress so far?
Any new hint or thought you'd like to share?
It would be great to hear from you!

[jeliasson]

@m3t Sorry for the lack of response. I have spoken to @Tobbe who will look closer on this. We do not believe this is related to Azure Active Directory specifically, as the skipAuth fire before reaching the auth provider, but hopefully Tobbe will be able to to find something.

[Tobbe]

@m3t Thank you for the reproduction repo. I totally misunderstood what the error was until I ran your code and saw the issue with my own eyes.

So, what's happening is that as soon as you load HomePage the useEffect will tell the Apollo client to make a gql query to the api side. Because the client can't know if the query requires auth or not it will always pass along an auth token. When the query reaches the api side the @skipAuth and @requireAuth directives determines if the token will be verified or not.

Because of this, no matter what kind of query/mutation you do, Redwood will always ask the auth provider client sdk for a token. And apparently, when it does, the Azure sdk will trigger the login screen.

[Tobbe]

Looking into the getToken implementation for Azure I see that it first tries client.acquireTokenSilent(). This fails, and so it does client.acqurieTokenRedirect().

The error it throws when it fails is

BrowserAuthError: no_account_error: No account object provided to acquireTokenSilent
and no active account has been set.
Please call setActiveAccount or provide an account on the request.

I'm afraid this is as far as I can take this on my own. @jeliasson do you think we can come up with a more clever way of determining if we should fall back to client.acquireTokenRedirect() or not? In this case it seems like we don't want to call that function at all. Or if anyone else have any ideas/input on how this should work, please share 🙂

[jeliasson]

@Tobbe Agreed, we need to add some logic for choosing a path if we get an exception from .acquireTokenSilent(). When looking at the MSAL documentation, we should probably do .acquireTokenRedirect() if the the error name is consent_required, interaction_required or login_required. I remember doing something like this in the initial implementation, but we had issues capturing the exception InteractionRequiredAuthError and settled with what we have right now.

We should work on this in the now decoupled auth package @redwoodjs/auth-azure-active-directory-web and introduce this change in a v4.x (minor) release, or should we try aim for the v4 release? You have a better understanding on the timeline here @Tobbe, so I defer the timing to you. Otherwise, maybe we can have some collaboration next week?

Docs

• https://github.com/AzureAD/microsoft-authentication-library-for-js/blob/dev/lib/msal-browser/docs/acquire-token.md
• https://learn.microsoft.com/en-us/azure/active-directory/develop/msal-error-handling-js#error-types
• https://learn.microsoft.com/en-us/azure/active-directory/develop/scenario-spa-acquire-token?tabs=javascript2#acquire-a-token-with-a-redirect
• https://learn.microsoft.com/en-us/azure/active-directory/develop/scenario-spa-sign-in?tabs=javascript2#sign-in-with-redirect

[Tobbe]

We're releasing v4 Wednesday next week unless something unforeseen comes up. But the change you're suggesting doesn't sound like a breaking change, so it can come in whenever and will just be included in the next release whichever one that might be. So if we fix it in the next few days it'll be in v4. Otherwise it'll be included in v4.1 (which we already have lined up for releasing Vite support)

I'll have a look at those docs and see if I can come up with something

[Tobbe]

Untested, but reading the docs it looks like something along these lines should catch all possibilities. But it sure is ugly :(

    getToken: async (options?: SilentRequest) => {
      // Default scopes if options is not passed
      const request = options || {
        scopes: ['openid', 'profile'],
      }

      // The recommended call pattern is to first try to call
      // acquireTokenSilent, and if it fails with an
      // InteractionRequiredAuthError, call acquireTokenRedirect
      // https://github.com/AzureAD/microsoft-authentication-library-for-js/blob/dev/lib/msal-browser/docs/acquire-token.md
      // NOTE: We are not catching the `InteractionRequiredAuthError`,
      // perhaps we can branch off `error.name` if this strategy doesn't work
      // properly.
      try {
        const token = await azureActiveDirectoryClient.acquireTokenSilent(
          request
        )
        return token.idToken
      } catch (err) {
        if (
          err instanceof InteractionRequiredAuthError ||
          (err instanceof AuthError &&
            (err.name === 'InteractionRequiredAuthError' ||
              err.errorCode === 'consent_required' ||
              err.errorCode === 'interaction_required' ||
              err.errorCode === 'login_required'))
        ) {
          // fallback to interaction when silent call fails
          return azureActiveDirectoryClient.acquireTokenRedirect(request)
        }
      }

      return null
    },

Does anyone know how I can force/trigger an InteractionRequiredAuthError?

[Tobbe]

These docs mentions extracting claims from the error and injecting into request. I tried to do that but got type errors. Is it something we should try to do here?

https://learn.microsoft.com/en-us/azure/active-directory/develop/msal-error-handling-js#conditional-access-and-claims-challenges

// extract, if exists, claims from error message
if (error.ErrorMessage.claims) {
  request.claimsRequest = JSON.stringify(error.ErrorMessage.claims);
}

The errors I get when I try that code are

1. It should be error.errorMessage instead of error.ErrorMessage
2. errorMessage is a string, and strings don't have claims
3. request is of type SilentRequest, and SilentRequest doesn't have claimsRequest

[Tobbe]

This works

      } catch (err) {
        if (err instanceof BrowserAuthError) {
          console.log('BrowserAuthError')
        }
      }

So I don't see why instanceof InteractionRequiredAuthError wouldn't work. Maybe they fixed it in a version of the azure sdk between when it was last tried and now.

[Tobbe]

I did the simplest implementation I could think of in #7386
Please run yarn rw upgrade -t canary in your projects (or create a new project and then upgrade) to verify that it works. If it doesn't, let me know and we'll reopen this issue.

[rafaelhutter]

@Tobbe Thank you so much for this!

[tilmann]

I can confirm that this fixes the issue. Thanks you so much @Tobbe . This has been a big blocker for a lot of features. 🚀

While testing I opened my first PR - documentation only - but hopefully more to come. 💙

[Tobbe]

I'm also working on some doc updates #7387

[tilmann]

Hi @Tobbe,

a quick question:
Am I correct that it is a bug that rw is aquiring a new token before each graphql request (adds 100-300ms to every interaction). This is what I see in the developer tools when clicking around the app

If you confirm that this is a bug I will open a new issue.

[jeliasson]

While we have a few Azure AD users in this issue; any of you have issues with iOS/Safari?

[jtoar]

@tilmann I don't believe this is a bug, but I'm guessing it could probably be done better given how #4739 (a similar Issue I think) resolved (via #5816).

I'm going way back here, so could be wrong, but I think the reason for fetching a new token before every request is #1576 and #1609.

[tilmann]

Thanks. @jtoar

This is very helpful. It seems like the behavior is the same as for dbAuth. As multiple requests after on interaction do not get a new token. Then I leave it as it is for now. And come back to it if I think we need to optimize for those 100-150ms.

[tilmann]

@jeliasson Tested with Safari on macos and ios and did not have any problems!

[jeliasson]

Just a follow up for anyone that might stumble upon this later;
I had issues with iOS WebKit browsers, and it was fixed in @azure/msal-browser on version 2.33.0.
AzureAD/microsoft-authentication-library-for-js#5625