jenkins-bot has submitted this change and it was merged.

Change subject: Retrieve group membership info when logging in
......................................................................


Retrieve group membership info when logging in

This is in preparation for bug T132834 (Semi-protected page cannot be edited).
The problem is that with the highly cached responses we get from RESTBase
the app doesn't get any user specific responses anymore.
In the case of a semi-protected page, it would return false, no matter as
what user one is logged in. Instead of taking that as the truth we could
consider the value of "editable" in the lead response to be "editable for
anonymous users". To be able to do that we need to retrieve the group
membership information for a logged in user first.

The membership info would only be available for newly logged in users.
If it's important that it works for already logged in users we could follow
up with another patch which checks if there is membership data when
downloading a page.

GroupMembershipClient:
Added another DataClient which automatically gets executed
when the login process is complete.

The login would be marked as failed if the retrieval of group membership
info fails. We could theoretically mark it as successful instead. This is
just so we can say that if a user is newly logged in and has the proper
membership she should be able to edit semi-protected pages.

The list=users request could provide more information in the future.
We just don't need more at this time.

Bug: T132834
Change-Id: Iccb8a482bbe4646ed3f625b7da8e3284a5a3f8d8
---
A app/src/main/java/org/wikipedia/login/GroupMembershipClient.java
M app/src/main/java/org/wikipedia/login/LoginClient.java
M app/src/main/java/org/wikipedia/login/User.java
M app/src/main/java/org/wikipedia/login/UserInfoStorage.java
M app/src/main/java/org/wikipedia/settings/Prefs.java
M app/src/main/java/org/wikipedia/settings/PrefsIoUtil.java
M app/src/main/res/values/preference_keys.xml
M app/src/test/java/org/wikipedia/login/UserTest.java
8 files changed, 253 insertions(+), 22 deletions(-)

Approvals:
  jenkins-bot: Verified
  Niedzielski: Looks good to me, approved



diff --git a/app/src/main/java/org/wikipedia/login/GroupMembershipClient.java 
b/app/src/main/java/org/wikipedia/login/GroupMembershipClient.java
new file mode 100644
index 0000000..b1229aa
--- /dev/null
+++ b/app/src/main/java/org/wikipedia/login/GroupMembershipClient.java
@@ -0,0 +1,126 @@
+package org.wikipedia.login;
+
+import org.wikipedia.Site;
+import org.wikipedia.dataclient.mwapi.MwQueryResponse;
+import org.wikipedia.dataclient.retrofit.MwCachedService;
+
+import com.google.gson.annotations.SerializedName;
+
+import retrofit2.Call;
+import retrofit2.Callback;
+import retrofit2.Response;
+import retrofit2.http.POST;
+import retrofit2.http.Query;
+
+import android.support.annotation.NonNull;
+import android.support.annotation.Nullable;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+/**
+ * Retrofit DataClient to retrieve implicit group membership information for a 
specific user.
+ */
+class GroupMembershipClient {
+    @NonNull private final MwCachedService<Service> cachedService = new 
MwCachedService<>(Service.class);
+
+    @Nullable private Call<MwQueryResponse<UserMemberships>> groupCall;
+
+    interface GroupMembershipCallback {
+        void success(@NonNull Set<String> result);
+        void error(@NonNull Throwable caught);
+    }
+
+    public void request(@NonNull final Site site, @NonNull final String 
userName,
+                        @NonNull final GroupMembershipCallback cb) {
+        cancel();
+
+        groupCall = cachedService.service(site).listUsers(userName);
+        groupCall.enqueue(new Callback<MwQueryResponse<UserMemberships>>() {
+            @Override
+            public void onResponse(Call<MwQueryResponse<UserMemberships>> call,
+                                   Response<MwQueryResponse<UserMemberships>> 
response) {
+                if (response.isSuccessful()) {
+                    final MwQueryResponse<UserMemberships> body = 
response.body();
+                    final UserMemberships query = body.query();
+                    if (query != null) {
+                        cb.success(query.getGroupsFor(userName));
+                    } else if (body.getError() != null) {
+                        cb.error(new LoginClient.LoginFailedException(
+                                "Failed to retrieve group membership data. "
+                                        + body.getError().toString()));
+                    } else {
+                        cb.error(new LoginClient.LoginFailedException(
+                                "Unexpected error trying to retrieve group 
membership data. "
+                                        + body.toString()));
+                    }
+                } else {
+                    cb.error(new 
LoginClient.LoginFailedException(response.message()));
+                }
+            }
+
+            @Override
+            public void onFailure(Call<MwQueryResponse<UserMemberships>> call, 
Throwable caught) {
+                cb.error(caught);
+            }
+        });
+    }
+
+    public void cancel() {
+        cancelTokenRequest();
+    }
+
+    private void cancelTokenRequest() {
+        if (groupCall == null) {
+            return;
+        }
+        groupCall.cancel();
+        groupCall = null;
+    }
+
+    private interface Service {
+
+        /** Request the implicit groups a user belongs to. */
+        @NonNull
+        @POST("w/api.php?format=json&formatversion=2&action=query"
+                + "&list=users&usprop=implicitgroups")
+        Call<MwQueryResponse<UserMemberships>> listUsers(
+                @Query("ususers") @NonNull String userName);
+    }
+
+    private static final class UserMemberships {
+        @SuppressWarnings("MismatchedReadAndWriteOfArray") 
@SerializedName("users") @NonNull
+        private List<ListUsersResponse> users = Collections.emptyList();
+
+        @NonNull Set<String> getGroupsFor(@NonNull String userName) {
+            if (!users.isEmpty()) {
+                for (ListUsersResponse user : users) {
+                    final Set<String> groups = user.getGroupsFor(userName);
+                    if (groups != null) {
+                        return groups;
+                    }
+                }
+            }
+            return Collections.emptySet();
+        }
+
+        private static final class ListUsersResponse {
+            @SerializedName("name") @Nullable private String name;
+
+            @SerializedName("implicitgroups") @Nullable private String[] 
implicitGroups;
+
+            @Nullable Set<String> getGroupsFor(@NonNull String userName) {
+                if (userName.equals(name) && implicitGroups != null) {
+                    Set<String> groups = new HashSet<>();
+                    groups.addAll(Arrays.asList(implicitGroups));
+                    return Collections.unmodifiableSet(groups);
+                } else {
+                    return null;
+                }
+            }
+        }
+    }
+}
diff --git a/app/src/main/java/org/wikipedia/login/LoginClient.java 
b/app/src/main/java/org/wikipedia/login/LoginClient.java
index 8bd56ca..9e68d36 100644
--- a/app/src/main/java/org/wikipedia/login/LoginClient.java
+++ b/app/src/main/java/org/wikipedia/login/LoginClient.java
@@ -21,13 +21,14 @@
 import android.support.annotation.Nullable;
 
 import java.io.IOException;
+import java.util.Set;
 
 /**
  * Responsible for making login related requests to the server.
  */
 public class LoginClient {
-    public static final String CLIENT_LOGIN = "clientlogin";
-    public static final String JSON = "json";
+    private static final String CLIENT_LOGIN = "clientlogin";
+    private static final String JSON = "json";
 
     @NonNull private final MwCachedService<Service> cachedService
             = new MwCachedService<>(Service.class);
@@ -53,11 +54,11 @@
             public void onResponse(Call<MwQueryResponse<LoginToken>> call,
                                    Response<MwQueryResponse<LoginToken>> 
response) {
                 if (response.isSuccessful()) {
-                    final MwQueryResponse<LoginToken> body = response.body();
-                    if (body.query() != null &&  body.query().getLoginToken() 
!= null) {
-                        login(site, userName, password, 
body.query().getLoginToken(), cb);
+                    MwQueryResponse<LoginToken> body = response.body();
+                    LoginToken query = body.query();
+                    if (query != null &&  query.getLoginToken() != null) {
+                        login(site, userName, password, query.getLoginToken(), 
cb);
                     } else if (body.getError() != null) {
-                        L.v(body.getError().toString());
                         cb.error(new IOException("Failed to retrieve login 
token. "
                                 + body.getError().toString()));
                     } else {
@@ -76,8 +77,9 @@
         });
     }
 
-    private void login(@NonNull Site site, @NonNull String userName, @NonNull 
final String password,
-                       @NonNull String loginToken, final LoginCallback cb) {
+    private void login(@NonNull final Site site, @NonNull final String 
userName,
+                       @NonNull final String password, @NonNull String 
loginToken,
+                       @NonNull final LoginCallback cb) {
         loginCall = cachedService.service(site).logIn(CLIENT_LOGIN, JSON, 
userName, password,
                 loginToken, Constants.WIKIPEDIA_URL);
         loginCall.enqueue(new Callback<LoginResponse>() {
@@ -87,9 +89,11 @@
                     LoginResponse loginResponse = response.body();
                     LoginResult loginResult = 
loginResponse.toLoginResult(password);
                     if (loginResult != null) {
-                        if (loginResult.pass()) {
-                            User.setUser(loginResult.getUser());
-                            cb.success(loginResult);
+                        if (loginResult.pass() && loginResult.getUser() != 
null) {
+                            // The server could do some transformations on 
user names, e.g. on some
+                            // wikis is uppercases the first letter.
+                            String actualUserName = 
loginResult.getUser().getUsername();
+                            getGroupMemberships(site, actualUserName, 
loginResult, cb);
                         } else {
                             cb.error(new 
LoginFailedException(loginResult.getMessage()));
                         }
@@ -105,6 +109,26 @@
             @Override
             public void onFailure(Call<LoginResponse> call, Throwable t) {
                 cb.error(t);
+            }
+        });
+    }
+
+    private void getGroupMemberships(@NonNull Site site, @NonNull String 
userName,
+                                     @NonNull final LoginResult loginResult,
+                                     @NonNull final LoginCallback cb) {
+        GroupMembershipClient groupClient = new GroupMembershipClient();
+        groupClient.request(site, userName, new 
GroupMembershipClient.GroupMembershipCallback() {
+            @Override
+            public void success(@NonNull Set<String> groups) {
+                final User user = loginResult.getUser();
+                User.setUser(new User(user, groups));
+                cb.success(loginResult);
+            }
+
+            @Override
+            public void error(@NonNull Throwable caught) {
+                L.e("Login suceeded but getting group information failed. " + 
caught);
+                cb.error(caught);
             }
         });
     }
@@ -151,7 +175,7 @@
     private static final class LoginToken {
         @SerializedName("tokens") private Tokens tokens;
 
-        @Nullable public String getLoginToken() {
+        @Nullable String getLoginToken() {
             return tokens == null ? null : tokens.loginToken;
         }
 
@@ -173,7 +197,7 @@
             return error;
         }
 
-        public LoginResult toLoginResult(String password) {
+        LoginResult toLoginResult(String password) {
             return clientLogin != null ? clientLogin.toLoginResult(password) : 
null;
         }
 
@@ -182,7 +206,7 @@
             @SerializedName("message") @Nullable private String message;
             @SerializedName("username") @Nullable private String userName;
 
-            public LoginResult toLoginResult(String password) {
+            LoginResult toLoginResult(String password) {
                 User user = null;
                 String userMessage = null;
                 if ("PASS".equals(status)) {
@@ -195,8 +219,8 @@
         }
     }
 
-    public class LoginFailedException extends Throwable {
-        public LoginFailedException(String message) {
+    public static class LoginFailedException extends Throwable {
+        LoginFailedException(String message) {
             super(message);
         }
     }
diff --git a/app/src/main/java/org/wikipedia/login/User.java 
b/app/src/main/java/org/wikipedia/login/User.java
index 6d7fd58..1bd191c 100644
--- a/app/src/main/java/org/wikipedia/login/User.java
+++ b/app/src/main/java/org/wikipedia/login/User.java
@@ -4,6 +4,9 @@
 import android.support.annotation.Nullable;
 import android.support.annotation.VisibleForTesting;
 
+import java.util.Collections;
+import java.util.Set;
+
 public class User {
     @Nullable private static UserInfoStorage STORAGE = new UserInfoStorage();
     @Nullable private static User CURRENT_USER;
@@ -45,11 +48,26 @@
     @NonNull private final String username;
     @NonNull private final String password;
     private final int userID;
+    @NonNull private final Set<String> groups;
 
     public User(@NonNull String username, @NonNull String password, int 
userID) {
+        this(username, password, userID, null);
+    }
+
+    public User(@NonNull User other, @Nullable Set<String> groups) {
+        this(other.username, other.password, other.userID, groups);
+    }
+
+    public User(@NonNull String username, @NonNull String password, int userID,
+                @Nullable Set<String> groups) {
         this.username = username;
         this.password = password;
         this.userID = userID;
+        if (groups != null) {
+            this.groups = Collections.unmodifiableSet(groups);
+        } else {
+            this.groups = Collections.emptySet();
+        }
     }
 
     @NonNull
@@ -65,4 +83,20 @@
     public int getUserID() {
         return userID;
     }
+
+    public boolean isAllowed(@NonNull String[] allowedGroups) {
+        for (String allowedGroup: allowedGroups) {
+            for (String group: groups) {
+                if (allowedGroup != null && allowedGroup.equals(group)) {
+                    return true;
+                }
+            }
+        }
+        return false;
+    }
+
+    @NonNull
+    Set<String> getGroupMemberships() {
+        return groups;
+    }
 }
diff --git a/app/src/main/java/org/wikipedia/login/UserInfoStorage.java 
b/app/src/main/java/org/wikipedia/login/UserInfoStorage.java
index 0a5cda7..a616d5d 100644
--- a/app/src/main/java/org/wikipedia/login/UserInfoStorage.java
+++ b/app/src/main/java/org/wikipedia/login/UserInfoStorage.java
@@ -11,6 +11,7 @@
         Prefs.setLoginUsername(user.getUsername());
         Prefs.setLoginPassword(user.getPassword());
         Prefs.setLoginUserId(user.getUserID());
+        Prefs.setLoginGroups(user.getGroupMemberships());
     }
 
     @Nullable
@@ -20,7 +21,8 @@
             return new User(
                     Prefs.getLoginUsername(),
                     Prefs.getLoginPassword(),
-                    Prefs.getLoginUserId()
+                    Prefs.getLoginUserId(),
+                    Prefs.getLoginGroups()
             );
         }
         return null;
@@ -30,5 +32,6 @@
         Prefs.removeLoginUsername();
         Prefs.removeLoginPassword();
         Prefs.removeLoginUserId();
+        Prefs.removeLoginGroups();
     }
 }
diff --git a/app/src/main/java/org/wikipedia/settings/Prefs.java 
b/app/src/main/java/org/wikipedia/settings/Prefs.java
index fdbbb52..2fbe5d2 100644
--- a/app/src/main/java/org/wikipedia/settings/Prefs.java
+++ b/app/src/main/java/org/wikipedia/settings/Prefs.java
@@ -31,11 +31,13 @@
 import static org.wikipedia.settings.PrefsIoUtil.getKey;
 import static org.wikipedia.settings.PrefsIoUtil.getLong;
 import static org.wikipedia.settings.PrefsIoUtil.getString;
+import static org.wikipedia.settings.PrefsIoUtil.getStringSet;
 import static org.wikipedia.settings.PrefsIoUtil.remove;
 import static org.wikipedia.settings.PrefsIoUtil.setBoolean;
 import static org.wikipedia.settings.PrefsIoUtil.setInt;
 import static org.wikipedia.settings.PrefsIoUtil.setLong;
 import static org.wikipedia.settings.PrefsIoUtil.setString;
+import static org.wikipedia.settings.PrefsIoUtil.setStringSet;
 
 /** Shared preferences utility for convenient POJO access. */
 public final class Prefs {
@@ -196,6 +198,19 @@
     }
 
     @Nullable
+    public static Set<String> getLoginGroups() {
+        return getStringSet(R.string.preference_key_login_groups, null);
+    }
+
+    public static void setLoginGroups(@Nullable Set<String> groups) {
+        setStringSet(R.string.preference_key_login_groups, groups);
+    }
+
+    public static void removeLoginGroups() {
+        remove(R.string.preference_key_login_groups);
+    }
+
+    @Nullable
     public static String getMruLanguageCodeCsv() {
         return getString(R.string.preference_key_language_mru, null);
     }
diff --git a/app/src/main/java/org/wikipedia/settings/PrefsIoUtil.java 
b/app/src/main/java/org/wikipedia/settings/PrefsIoUtil.java
index f85b832..047bd58 100644
--- a/app/src/main/java/org/wikipedia/settings/PrefsIoUtil.java
+++ b/app/src/main/java/org/wikipedia/settings/PrefsIoUtil.java
@@ -10,6 +10,8 @@
 
 import org.wikipedia.WikipediaApp;
 
+import java.util.Set;
+
 /** Shared preferences input / output utility providing set* functionality 
that writes to SP on the
  * client's behalf, IO without client supplied {@link Context}, and wrappers 
for using string
  * resources as keys, and unifies SP access. */
@@ -21,6 +23,15 @@
 
     public static void setString(@StringRes int id, @Nullable String value) {
         setString(getKey(id), value);
+    }
+
+    @Nullable
+    public static Set<String> getStringSet(@StringRes int id, @Nullable 
Set<String> defaultValue) {
+        return getStringSet(getKey(id), defaultValue);
+    }
+
+    public static void setStringSet(@StringRes int id, @Nullable Set<String> 
value) {
+        setStringSet(getKey(id), value);
     }
 
     public static long getLong(@StringRes int id, long defaultValue) {
@@ -56,6 +67,15 @@
         edit().putString(key, value).apply();
     }
 
+    @Nullable
+    public static Set<String> getStringSet(String key, @Nullable Set<String> 
defaultValue) {
+        return getPreferences().getStringSet(key, defaultValue);
+    }
+
+    public static void setStringSet(String key, @Nullable Set<String> value) {
+        edit().putStringSet(key, value).apply();
+    }
+
     public static long getLong(String key, long defaultValue) {
         return getPreferences().getLong(key, defaultValue);
     }
diff --git a/app/src/main/res/values/preference_keys.xml 
b/app/src/main/res/values/preference_keys.xml
index dda1631..1a78135 100644
--- a/app/src/main/res/values/preference_keys.xml
+++ b/app/src/main/res/values/preference_keys.xml
@@ -31,6 +31,7 @@
     <string name="preference_key_login_username">username</string>
     <string name="preference_key_login_password">password</string>
     <string name="preference_key_login_user_id">userID</string>
+    <string name="preference_key_login_groups">groups</string>
     <string 
name="preference_key_show_developer_settings">showDeveloperSettings</string>
     <string name="preference_key_last_run_time_format">%s-lastrun</string>
     <string name="preference_key_tabs">tabs</string>
diff --git a/app/src/test/java/org/wikipedia/login/UserTest.java 
b/app/src/test/java/org/wikipedia/login/UserTest.java
index 9c1f930..0e3fecb 100644
--- a/app/src/test/java/org/wikipedia/login/UserTest.java
+++ b/app/src/test/java/org/wikipedia/login/UserTest.java
@@ -6,6 +6,11 @@
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.nullValue;
@@ -13,22 +18,25 @@
 @RunWith(TestRunner.class)
 public class UserTest {
     private static final int USER_ID = 333;
+    private static final Set<String> GROUPS
+            = Collections.unmodifiableSet(new HashSet<>(Arrays.asList("*", 
"user", "autoconfirmed")));
 
     @Before
     public void setUp() {
         User.disableStorage();
 
-        User user = new User("name", "pwd", USER_ID);
+        User user = new User("name", "pwd", USER_ID, GROUPS);
         User.setUser(user);
     }
 
     @Test
     public void testUserLogin() {
-        User user2 = User.getUser();
+        User user = User.getUser();
         //noinspection ConstantConditions
-        assertThat(user2.getUsername(), is("name"));
-        assertThat(user2.getPassword(), is("pwd"));
-        assertThat(user2.getUserID(), is(USER_ID));
+        assertThat(user.getUsername(), is("name"));
+        assertThat(user.getPassword(), is("pwd"));
+        assertThat(user.getUserID(), is(USER_ID));
+        assertThat(user.getGroupMemberships(), is(GROUPS));
     }
 
     @Test

-- 
To view, visit https://gerrit.wikimedia.org/r/313031
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Iccb8a482bbe4646ed3f625b7da8e3284a5a3f8d8
Gerrit-PatchSet: 19
Gerrit-Project: apps/android/wikipedia
Gerrit-Branch: master
Gerrit-Owner: BearND <bsitzm...@wikimedia.org>
Gerrit-Reviewer: BearND <bsitzm...@wikimedia.org>
Gerrit-Reviewer: Brion VIBBER <br...@wikimedia.org>
Gerrit-Reviewer: Dbrant <dbr...@wikimedia.org>
Gerrit-Reviewer: Mholloway <mhollo...@wikimedia.org>
Gerrit-Reviewer: Niedzielski <sniedziel...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to