Skip to content

Commit 8440b39

Browse files
committed
Protect social unlink from account lockout
Keep social unlink disabled when removing the last available sign-in method. Expose the capability to the settings UI and enforce the same rule on the unbind endpoint. Add frontend and controller coverage for the allowed and blocked cases.
1 parent 5546ec2 commit 8440b39

7 files changed

Lines changed: 115 additions & 12 deletions

File tree

apps/codebattle/assets/js/__tests__/UserSettings.test.jsx

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ const preloadedState = {
4949
name: "Diman",
5050
lang: "ts",
5151
avatarUrl: "/assets/images/logo.svg",
52+
canUnlinkSocial: false,
5253
discordName: null,
5354
discordId: null,
5455
error: "",
@@ -216,4 +217,56 @@ describe("UserSettings test cases", () => {
216217

217218
expect(await findByRole("alert")).toHaveClass("alert-danger");
218219
});
220+
221+
test("unlink button is disabled when it is the last sign-in method", () => {
222+
const customStore = configureStore({
223+
reducer,
224+
preloadedState: {
225+
user: {
226+
settings: {
227+
...preloadedState.user.settings,
228+
githubId: 19,
229+
githubName: "octocat",
230+
canUnlinkSocial: false,
231+
discordId: null,
232+
discordName: null,
233+
},
234+
},
235+
},
236+
});
237+
238+
const { getByRole } = setup(
239+
<Provider store={customStore}>
240+
<UserSettings />
241+
</Provider>,
242+
);
243+
244+
expect(getByRole("button", { name: "Unlink Github" })).toBeDisabled();
245+
});
246+
247+
test("unlink button stays enabled when another sign-in method exists", () => {
248+
const customStore = configureStore({
249+
reducer,
250+
preloadedState: {
251+
user: {
252+
settings: {
253+
...preloadedState.user.settings,
254+
githubId: 19,
255+
githubName: "octocat",
256+
canUnlinkSocial: true,
257+
discordId: null,
258+
discordName: null,
259+
},
260+
},
261+
},
262+
});
263+
264+
const { getByRole } = setup(
265+
<Provider store={customStore}>
266+
<UserSettings />
267+
</Provider>,
268+
);
269+
270+
expect(getByRole("button", { name: "Unlink Github" })).toBeEnabled();
271+
});
219272
});

apps/codebattle/assets/js/widgets/pages/settings/UserSettings.jsx

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,6 @@ function Notification({ notification, onClose }) {
6565
}
6666

6767
function SocialButtons({ settings }) {
68-
const onlyOneProviderLinked =
69-
providers.filter((provider) => !!settings[mapUserPropNameByProviderName[provider]]).length ===
70-
1;
71-
7268
return providers.map((provider) => {
7369
const providerPropName = mapUserPropNameByProviderName[provider];
7470
const isLinked = !!settings[providerPropName];
@@ -87,7 +83,7 @@ function SocialButtons({ settings }) {
8783
data-method="delete"
8884
data-csrf={csrfToken}
8985
data-to={`/auth/${provider}`}
90-
disabled={onlyOneProviderLinked}
86+
disabled={!settings.canUnlinkSocial}
9187
>
9288
{`Unlink ${formatedProviderName}`}
9389
</button>

apps/codebattle/lib/codebattle/user.ex

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,10 @@ defmodule Codebattle.User do
263263
|> Repo.update()
264264
end
265265

266+
def can_unlink_social?(user) do
267+
available_login_methods_count(user) > 1
268+
end
269+
266270
def update_subscription_type(user_id, type) do
267271
user_id
268272
|> get!()
@@ -303,6 +307,20 @@ defmodule Codebattle.User do
303307

304308
def subscription_types, do: @subscription_types
305309

310+
defp available_login_methods_count(user) do
311+
Enum.count(
312+
[
313+
present?(user.github_id),
314+
present?(user.discord_id),
315+
present?(user.firebase_uid) or present?(user.password_hash),
316+
present?(user.external_oauth_id)
317+
],
318+
& &1
319+
)
320+
end
321+
322+
defp present?(value), do: not is_nil(value) and value != ""
323+
306324
defp assign_clan(changeset, %{:clan => clan}, _user_id) when clan in ["", nil],
307325
do: change(changeset, %{clan: nil, clan_id: nil})
308326

apps/codebattle/lib/codebattle_web/controllers/api/v1/settings_controller.ex

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ defmodule CodebattleWeb.Api.V1.SettingsController do
1111
name: current_user.name,
1212
clan: current_user.clan,
1313
locale: current_user.locale,
14+
can_unlink_social: User.can_unlink_social?(current_user),
15+
discord_id: current_user.discord_id,
16+
discord_name: current_user.discord_name,
1417
github_id: current_user.github_id,
1518
github_name: current_user.github_name,
1619
sound_settings: current_user.sound_settings,
@@ -32,6 +35,7 @@ defmodule CodebattleWeb.Api.V1.SettingsController do
3235
name: user.name,
3336
clan: user.clan,
3437
locale: user.locale,
38+
can_unlink_social: User.can_unlink_social?(user),
3539
sound_settings: user.sound_settings,
3640
lang: user.lang,
3741
style_lang: user.style_lang,

apps/codebattle/lib/codebattle_web/controllers/auth_bind_controller.ex

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -64,15 +64,19 @@ defmodule CodebattleWeb.AuthBindController do
6464
end
6565

6666
def unbind(conn, params) do
67-
current_user = conn.assigns.current_user
67+
current_user = Codebattle.Repo.reload(conn.assigns.current_user)
6868

6969
case_result =
70-
case params["provider"] do
71-
"github" ->
72-
GithubUser.unbind(current_user)
73-
74-
"discord" ->
75-
DiscordUser.unbind(current_user)
70+
if Codebattle.User.can_unlink_social?(current_user) do
71+
case params["provider"] do
72+
"github" ->
73+
GithubUser.unbind(current_user)
74+
75+
"discord" ->
76+
DiscordUser.unbind(current_user)
77+
end
78+
else
79+
{:error, gettext("You need at least one authentication method to sign in.")}
7680
end
7781

7882
case case_result do

apps/codebattle/lib/codebattle_web/plugs/assign_gon.ex

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ defmodule CodebattleWeb.Plugs.AssignGon do
3535
user
3636
|> Map.take([
3737
:avatar_url,
38+
:can_unlink_social,
3839
:clan,
3940
:clan_id,
4041
:discord_avatar,
@@ -65,6 +66,7 @@ defmodule CodebattleWeb.Plugs.AssignGon do
6566
:subscription_type,
6667
:sound_settings
6768
])
69+
|> Map.put(:can_unlink_social, Codebattle.User.can_unlink_social?(user))
6870
|> Map.put(:is_admin, Codebattle.User.admin?(user))
6971
end
7072
end

apps/codebattle/test/codebattle_web/controllers/auth_bind_controller_test.exs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,5 +120,31 @@ defmodule CodebattleWeb.AuthBindControllerTest do
120120
assert user.github_id == nil
121121
assert user.github_name == nil
122122
end
123+
124+
test "does not unbind the last available social sign-in method", %{conn: conn} do
125+
user =
126+
insert(:user,
127+
github_id: 42,
128+
github_name: "octocat",
129+
discord_id: nil,
130+
discord_name: nil,
131+
discord_avatar: nil,
132+
firebase_uid: nil,
133+
password_hash: nil,
134+
external_oauth_id: nil
135+
)
136+
137+
conn =
138+
conn
139+
|> put_session(:user_id, user.id)
140+
|> delete("/auth/github")
141+
142+
user = Repo.reload!(user)
143+
144+
assert user.github_id == 42
145+
assert user.github_name == "octocat"
146+
assert redirected_to(conn) == "/settings"
147+
assert Phoenix.Flash.get(conn.assigns.flash, :danger) =~ "at least one authentication method"
148+
end
123149
end
124150
end

0 commit comments

Comments
 (0)