Skip to content

[New] Authenticate with token #613

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

Merged
merged 15 commits into from
May 19, 2025
Merged

Conversation

des12437
Copy link
Contributor

@des12437 des12437 commented May 13, 2025

Description

This PR implements Authenticate with token in Cloud and Portal category.
URL to README: URL

Linked Issue(s)

  • swift/issues/6858

How To Test

  • Open the sample and sign in with valid credentials, verify the secured layer loads.
  • Sign in with invalid credentials and verify that an error message is shown.

Screenshots

authenticate-with-token1 authenticate-with-token2

@des12437 des12437 self-assigned this May 13, 2025
@des12437 des12437 requested review from njarecha, a team and mhdostal and removed request for a team May 13, 2025 22:17
@des12437 des12437 marked this pull request as ready for review May 13, 2025 22:17
mhdostal
mhdostal previously approved these changes May 14, 2025
Copy link
Member

@mhdostal mhdostal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good once the questions are answered.

Copy link
Collaborator

@njarecha njarecha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done with first pass

Copy link
Collaborator

@njarecha njarecha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make changes based on other auth samples for setting up and tearing down authenticator.

@des12437 des12437 requested a review from njarecha May 15, 2025 20:59
njarecha
njarecha previously approved these changes May 15, 2025
Copy link
Collaborator

@njarecha njarecha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for making all the changes

@des12437 des12437 requested review from a team, rolson and zkline101 and removed request for mhdostal, a team and zkline101 May 15, 2025 21:38
@des12437
Copy link
Contributor Author

Removed Mark as a reviewer as he is out of office today.

njarecha
njarecha previously approved these changes May 16, 2025
njarecha
njarecha previously approved these changes May 16, 2025
Copy link
Collaborator

@rolson rolson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only thing that needs to authenticate is the traffic layer. I'll have to think about a way to simplify this.

@rolson
Copy link
Collaborator

rolson commented May 16, 2025

I think this sample can be simplified with something like this, which just looks at the layer view state and shows an error:

struct AuthenticateWithTokenView: View {
    /// The authenticator to handle authentication challenges.
    @StateObject private var authenticator = Authenticator()
    
    @State private var map = {
        // The portal to authenticate with named user.
        let portal = Portal(url: .portal, connection: .authenticated)
        
        // The portal item to be displayed on the map.
        let portalItem = PortalItem(
            portal: portal,
            id: .trafficMap
        )
        
        // Creates map with portal item.
        return Map(item: portalItem)
    }()
    
    @State private var error: Error?
    
    var body: some View {
        MapView(map: map)
            .onLayerViewStateChanged { layer, layerViewState in
                if layerViewState.status == .error {
                    error = layer.loadError
                }
            }
            .errorAlert(presentingError: $error)
            .authenticator(authenticator)
            .onAppear {
                setupAuthenticator()
            }
            .onDisappear {
                Task {
                    // Reset the challenge handlers and clear credentials
                    // when the view disappears so that user is prompted to enter
                    // credentials every time the sample is run, and to clean
                    // the environment for other samples.
                    await teardownAuthenticator()
                }
            }
    }
}

You might want to fixup the error alert to say what you are looking for it to say with the auth error. And you could also check the layer name or that it's the first operational layer before setting the error.

@des12437
Copy link
Contributor Author

@rolson Thanks I applied your suggestion.

you could also check the layer name or that it's the first operational layer before setting the error.

This map has only one operational layer, so checking the name may not be necessary.

You might want to fixup the error alert to say what you are looking for it to say with the auth error.

Here is what the error alerts looks like when the user cancels authentication and when authentication fails due to invalid credentials respectively:

@des12437 des12437 requested review from rolson and njarecha May 16, 2025 22:35
@rolson
Copy link
Collaborator

rolson commented May 16, 2025

You can fixup the error to show what you want if you like the strings you had before. I just didn't go through that exercise.

@rolson
Copy link
Collaborator

rolson commented May 16, 2025

This map has only one operational layer, so checking the name may not be necessary.

Well, if you are only looking to show an error when the operational layer fails, then you might want to check that the layer is the first operational layer in your map before setting the error.

@des12437
Copy link
Contributor Author

You can fixup the error to show what you want if you like the strings you had before. I just didn't go through that exercise.

I used a string before since a standalone view with Text was shown for the error. The refactor uses an errorAlert, which requires an Error. I'm ok leaving this as-is and only using an alert for the error.

@des12437 des12437 merged commit 4502d48 into v.next May 19, 2025
1 check passed
@des12437 des12437 deleted the destiny/Authenticate-with-token branch May 19, 2025 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants