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

Enable Login v2 #2908

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

Enable Login v2 #2908

wants to merge 22 commits into from

Conversation

mpivchev
Copy link
Collaborator

@mpivchev mpivchev commented Apr 30, 2024

This enables Login v2. Code was already available

Note: The log in still happens in an in-app browser, IMO this is a better UX. @tobiasKaminsky :

Simulator.Screen.Recording.-.iPhone.15.-.2024-04-30.at.13.40.57.mp4

@marinofaggiana We can remove all v1 code with removal of oldest supported NC version

Signed-off-by: Milen Pivchev <[email protected]>
@@ -68,7 +68,7 @@ let userAgent: String = {
@objc public var use_themingColor: Bool = true
@objc public var use_themingLogo: Bool = false
@objc public var use_storeLocalAutoUploadAll: Bool = false
@objc public var use_loginflowv2: Bool = false // Don't touch me !!
@objc public var use_loginflowv2: Bool = true // Don't touch me !!
Copy link
Collaborator Author

@mpivchev mpivchev Apr 30, 2024

Choose a reason for hiding this comment

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

@marinofaggiana The plan is to use v2 by default from now on, and v1 will be obsolete with removal of oldest supported NC version. Maybe we can remove this? Not sure if the brander will break

Copy link
Member

Choose a reason for hiding this comment

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

yes I created this switch years ago to test v2

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is it ok to remove it?

@mpivchev mpivchev linked an issue Apr 30, 2024 that may be closed by this pull request
@mpivchev
Copy link
Collaborator Author

mpivchev commented May 2, 2024

Added the option to do the flow via external browser @tobiasKaminsky

@@ -47,6 +47,9 @@ class NCLoginWeb: UIViewController {
var loginFlowV2Endpoint = ""
var loginFlowV2Login = ""

// Opens the login URL in external browser instead of in app. User must manually go back to the app.
let loginFlowv2ExternalBrowser = false
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@marinofaggiana Please test with this enabled and disabled

NextcloudKit.shared.getLoginFlowV2Poll(token: self.loginFlowV2Token, endpoint: self.loginFlowV2Endpoint) { server, loginName, appPassword, _, error in
if error == .success, let server, let loginName, let appPassword {
self.createAccount(server: server, username: loginName, password: appPassword)
}
Copy link
Member

Choose a reason for hiding this comment

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

Simulator Screenshot - iPhone 15 Pro Max - 2024-05-15 at 13 30 34

If error write a message and goBack ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I stopped using NCLoginWeb for v2 and this screen stopped appearing. Now we only use NCLogin. NCLoginWeb can be renamed to NCLoginProviders as now only external providers use it.

@mpivchev
Copy link
Collaborator Author

mpivchev commented May 15, 2024

@marinofaggiana Please check openLogin function in AppDelegate, as that still calls NCLoginWeb

Signed-off-by: Milen Pivchev <[email protected]>
Signed-off-by: Milen Pivchev <[email protected]>
Signed-off-by: Milen Pivchev <[email protected]>
Signed-off-by: Milen Pivchev <[email protected]>
Signed-off-by: Milen Pivchev <[email protected]>
Signed-off-by: Milen Pivchev <[email protected]>
Signed-off-by: Milen Pivchev <[email protected]>
Signed-off-by: Milen Pivchev <[email protected]>
Signed-off-by: Milen Pivchev <[email protected]>
Signed-off-by: Milen Pivchev <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Login flow v2
2 participants