[Android] OAuth2 flow with browser V2 #9044

strump · 2024-08-21T13:36:47Z

strump commented

2024-08-21 13:36:47 +00:00

N.B. New login flow is enabled only for "google" builds! F-Droid and Web builds should work as previously.

How OAuth2 flow should work (happy path):

OsmLoginFragment - User pushes "Login to OpenStreetMap" button
OsmLoginFragment - triggers browser
Browser - OSM.org login flow
Browser - redirects to om://oauth2/osm/callback?code=XXX
MwmActivity - handles om:// URI and extracts code value
MwmActivity - calls OsmLoginActivity with code in extras
OsmLoginFragment - calls native code to get OAuth2 token using code
OsmLoginFragment - calls native code to get username using OAuth2 token
OsmLoginFragment - redirects to ProfileActivity

https://github.com/user-attachments/assets/94f51fca-4fac-45b8-94ae-7495731ed93e

This PR does the same as #8825 but with no UI changes.

**N.B.** New login flow is enabled only for "google" builds! F-Droid and Web builds should work as previously. How OAuth2 flow should work (happy path): * `OsmLoginFragment` - User pushes "Login to OpenStreetMap" button * `OsmLoginFragment` - triggers browser * `Browser` - OSM.org login flow * `Browser` - redirects to `om://oauth2/osm/callback?code=XXX` * `MwmActivity` - handles `om://` URI and extracts code value * `MwmActivity` - calls OsmLoginActivity with code in extras * `OsmLoginFragment` - calls native code to get OAuth2 token using code * `OsmLoginFragment` - calls native code to get username using OAuth2 token * `OsmLoginFragment` - redirects to `ProfileActivity` https://github.com/user-attachments/assets/94f51fca-4fac-45b8-94ae-7495731ed93e This PR does the same as #8825 but with no UI changes.

biodranik (Migrated from github.com) reviewed 2024-08-21 14:16:51 +00:00

biodranik (Migrated from github.com) left a comment

Thanks! It works in the emulator, a bit of nit code styling and we can merge into the master to beta-test on many users.

android/app/src/main/java/app/organicmaps/editor/OsmLoginFragment.java

					
				@ -48,15 +50,41 @@ public class OsmLoginFragment extends BaseMwmToolbarFragment

				    mLoginInput = view.findViewById(R.id.osm_username);

				    mPasswordInput = view.findViewById(R.id.osm_password);

biodranik (Migrated from github.com) commented

2024-08-21 13:56:28 +00:00

Does this code work for a non-google flavor?

biodranik (Migrated from github.com) commented

2024-08-21 14:01:06 +00:00

Can storing this variable as a member be avoided?

android/app/src/main/java/app/organicmaps/editor/OsmLoginFragment.java

					
				@ -113,4 +146,27 @@ public class OsmLoginFragment extends BaseMwmToolbarFragment

				      startActivity(new Intent(requireContext(), ProfileActivity.class));

				    requireActivity().finish();

				  }

biodranik (Migrated from github.com) commented

2024-08-21 14:02:10 +00:00

      ThreadPool.getWorker().execute(() ->
      {

```suggestion ThreadPool.getWorker().execute(() -> { ```

biodranik (Migrated from github.com) commented

2024-08-21 14:02:25 +00:00

        UiThread.run(() ->
        {

```suggestion UiThread.run(() -> { ```

android/app/src/main/java/app/organicmaps/editor/OsmLoginFragment.java

					
				@ -116,1 +169,4 @@

				      });

				    }

				  }

				}

biodranik (Migrated from github.com) commented

2024-08-21 14:01:18 +00:00

```suggestion ```

editor/osm_auth.cpp

biodranik (Migrated from github.com) commented

2024-08-21 14:03:47 +00:00

   auto const requestTokenUrl = m_baseUrl + "/oauth2/authorize";

```suggestion auto const requestTokenUrl = m_baseUrl + "/oauth2/authorize"; ```

biodranik (Migrated from github.com) commented

2024-08-21 14:04:03 +00:00

or std::string

   auto const requestTokenQuery = BuildPostRequest(
   {

or std::string ```suggestion auto const requestTokenQuery = BuildPostRequest( { ```

editor/osm_auth.cpp

					
				@ -283,3 +288,4 @@

				string OsmOAuth::FinishAuthorization(string const & oauth2code) const

				{

				  auto params = BuildPostRequest({

biodranik (Migrated from github.com) commented

2024-08-21 14:02:49 +00:00

std::string OsmOAuth::BuildOAuth2Url() const

```suggestion std::string OsmOAuth::BuildOAuth2Url() const ```

strump reviewed 2024-08-21 14:37:44 +00:00

android/app/src/main/java/app/organicmaps/editor/OsmLoginFragment.java

					
				@ -48,15 +50,41 @@ public class OsmLoginFragment extends BaseMwmToolbarFragment

				    mLoginInput = view.findViewById(R.id.osm_username);

				    mPasswordInput = view.findViewById(R.id.osm_password);

strump commented

2024-08-21 14:37:44 +00:00

Yes. OAuth2 callback URIs are handled by all flavors. And if user have two versions installed (GooglePlay and F-Droid) he will see "Select which app to use" because om:// URIs are handled by all flavors.

Yes. OAuth2 callback URIs are handled by all flavors. And if user have two versions installed (GooglePlay and F-Droid) he will see "Select which app to use" because `om://` URIs are handled by all flavors.

strump reviewed 2024-08-21 14:44:10 +00:00

editor/osm_auth.cpp

strump commented

2024-08-21 14:44:10 +00:00

This variable couldn't be const because .append("...") method fails.

This variable couldn't be `const` because `.append("...")` method fails.

strump reviewed 2024-08-21 14:48:01 +00:00

editor/osm_auth.cpp

strump commented

2024-08-21 14:48:01 +00:00

Fixed

strump reviewed 2024-08-21 14:49:10 +00:00

editor/osm_auth.cpp

					
				@ -283,3 +288,4 @@

				string OsmOAuth::FinishAuthorization(string const & oauth2code) const

				{

				  auto params = BuildPostRequest({

strump commented

2024-08-21 14:49:10 +00:00

In editor/osm_auth.cpp file all methods have string return type. Should I change all methods to std::string?

In `editor/osm_auth.cpp` file all methods have `string` return type. Should I change all methods to `std::string`?

strump reviewed 2024-08-21 14:49:15 +00:00

android/app/src/main/java/app/organicmaps/editor/OsmLoginFragment.java

					
				@ -113,4 +146,27 @@ public class OsmLoginFragment extends BaseMwmToolbarFragment

				      startActivity(new Intent(requireContext(), ProfileActivity.class));

				    requireActivity().finish();

				  }

strump commented

2024-08-21 14:49:14 +00:00

Fixed

strump reviewed 2024-08-21 14:49:19 +00:00

android/app/src/main/java/app/organicmaps/editor/OsmLoginFragment.java

					
				@ -113,4 +146,27 @@ public class OsmLoginFragment extends BaseMwmToolbarFragment

				      startActivity(new Intent(requireContext(), ProfileActivity.class));

				    requireActivity().finish();

				  }

strump commented

2024-08-21 14:49:18 +00:00

Fixed

strump reviewed 2024-08-21 14:50:16 +00:00

android/app/src/main/java/app/organicmaps/editor/OsmLoginFragment.java

					
				@ -48,15 +50,41 @@ public class OsmLoginFragment extends BaseMwmToolbarFragment

				    mLoginInput = view.findViewById(R.id.osm_username);

				    mPasswordInput = view.findViewById(R.id.osm_password);

strump commented

2024-08-21 14:50:16 +00:00

I introduced String readOAuth2CodeFromArguments() method to get rid of the field.

I introduced `String readOAuth2CodeFromArguments()` method to get rid of the field.

biodranik (Migrated from github.com) reviewed 2024-08-21 14:55:50 +00:00

editor/osm_auth.cpp

biodranik (Migrated from github.com) commented

2024-08-21 14:55:50 +00:00

My bad, sorry.

biodranik (Migrated from github.com) reviewed 2024-08-21 14:56:24 +00:00

editor/osm_auth.cpp

					
				@ -283,3 +288,4 @@

				string OsmOAuth::FinishAuthorization(string const & oauth2code) const

				{

				  auto params = BuildPostRequest({