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

feat: added support for visionOS platform #859

Open
wants to merge 22 commits into
base: master
Choose a base branch
from

Conversation

desusai7
Copy link
Contributor

@desusai7 desusai7 commented Jun 20, 2024

πŸ“‹ Changes

feat: added support for visionOS platform

  • Added a framework target for visionOS platform and a unit test target for the same framework and a visionOS sample app to try the framework.
  • Made changes in the existing code to support visionOS whereever as Required, few examples include ASProvider.swift, Auth0WebAuth.swift & MobileWebAuth.swift.
  • Upgraded Quick & Nimble to latest versions in order to support visionOS.
  • Migrated the usages of sharedExamples from Quick to Behavior pattern as its the recommended, and at the same time we are facing issues with it on tvOS platform.
  • Added cwlPreConditionTesting package via SPM as latest versions of Quick & Nimble needed it.
  • Replaced the usage of OHHTTPStubs with our own logic for stubbing the network requests as OHHTTPStubs does not support visionOS platform yet.
  • Dropped support for Xcode 14 and Swift versions [5.7, 5.8] as they came along with Xcode 14 and the minimum version of swift required now is 5.9
  • Dropped support for iOS 13 & tvOS 13 as per our support policy to support only the last 4 major versions of a platform including the current major version.

βœ… Checks

  • All new/changed/fixed functionality is covered by tests (or N/A)
  • I have added documentation for all new/changed functionality (or N/A)

πŸ“Ž References

Auth0.swift#823

🎯 Testing

  • Performed E2E testing on visionOS sample app using WebAuth via ASWebAuthenticationSession, and not on SFSafariViewController as we are not supporting it as user agent on visionOS platform as its opening as a new tab in the safari app instead of a pop-up in the app.
  • Ensured all the unit tests across all the platforms including visionOS are working as expected.

@desusai7 desusai7 self-assigned this Jun 20, 2024
desusai7 and others added 19 commits June 21, 2024 02:21
@desusai7 desusai7 marked this pull request as ready for review July 4, 2024 13:27
@desusai7 desusai7 requested a review from a team as a code owner July 4, 2024 13:27
@desusai7 desusai7 enabled auto-merge (squash) July 4, 2024 13:28
func presentationAnchor(for session: ASWebAuthenticationSession) -> ASPresentationAnchor {
if let windowScene = UIApplication.shared()?.connectedScenes.first(where: { $0.activationState == .foregroundActive }) as? UIWindowScene {
return windowScene.windows.last(where: \.isKeyWindow) ?? ASPresentationAnchor()
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: the else is not necessary here because of the early return on L24.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, modified it accordingly

Comment on lines 15 to 16
static var authentication = Auth0.authentication(clientId: clientId, domain: domain)
static var validatorContext = IDTokenValidatorContext(issuer: URL.httpsURL(from: domain).absoluteString,
Copy link
Contributor

Choose a reason for hiding this comment

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

Could these be static constants?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, updated them

Comment on lines 500 to 502
beforeEach {

}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not necessary, removed it now

stub(condition: isToken(Domain) && hasAtLeast(["refresh_token": RefreshToken, key: value]) && hasHeader(key, value: "bar")) { _ in
return authResponse(accessToken: NewAccessToken, idToken: NewIdToken, refreshToken: NewRefreshToken, expiresIn: ExpiresIn)
}
NetworkStub.addStub(condition: { $0.isToken(Domain) && $0.hasAtLeast(["refresh_token": RefreshToken, key: value])}, response: authResponse(accessToken: NewAccessToken, idToken: NewIdToken, refreshToken: NewRefreshToken, expiresIn: ExpiresIn))
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems to be missing a hasHeader check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, thank you for pointing it out, added it back now

typealias RequestResponse = (URLRequest) -> (Data?, URLResponse?, Error?)

struct NetworkStub {
static var stubs: [(condition: RequestCondition, response: RequestResponse)] = []
Copy link
Contributor

@Widcket Widcket Jul 4, 2024

Choose a reason for hiding this comment

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

Should this be private(set)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, made its setter private now

func apiSuccessResponse(json: [AnyHashable: Any] = [:]) -> RequestResponse {
return { request in
let data = try! JSONSerialization.data(withJSONObject: json, options: [])
let response = HTTPURLResponse(url: request.url!, statusCode: APISuccessStatusCode, httpVersion: nil, headerFields: APIResponseHeaders)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we force-unwrap the response here as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, missed it, added it now

@desusai7 desusai7 requested a review from Widcket July 5, 2024 04:45
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.

None yet

2 participants