fix(instance): add needs_setup so admin-less instances aren't treated as fresh
The frontend keyed first-run setup off a null InstanceProfile.admin, but a null admin only means "no admin-role user exists" — which also happens on a populated instance that has lost all its admins. Such an instance was wrongly redirected to signup, where the new account is created as a normal user (the first-user promotion only triggers when there are zero users), leaving the instance permanently admin-less. Add an explicit InstanceProfile.needs_setup derived from user count == 0, and switch the signup redirect and host tip to use it. admin stays for display only.
This commit is contained in:
parent
6eb17864df
commit
96cb65320b
8 changed files with 96 additions and 18 deletions
|
|
@ -68,12 +68,19 @@ message InstanceProfile {
|
||||||
// Instance URL is the URL of the instance.
|
// Instance URL is the URL of the instance.
|
||||||
string instance_url = 6;
|
string instance_url = 6;
|
||||||
|
|
||||||
// The first administrator who set up this instance.
|
// The first administrator who set up this instance, for display purposes.
|
||||||
// When null, instance requires initial setup (creating the first admin account).
|
// May be null on an instance that has lost all admins; use needs_setup to
|
||||||
|
// determine whether initial setup is actually required.
|
||||||
User admin = 7;
|
User admin = 7;
|
||||||
|
|
||||||
// Commit is the current build commit of instance.
|
// Commit is the current build commit of instance.
|
||||||
string commit = 8;
|
string commit = 8;
|
||||||
|
|
||||||
|
// NeedsSetup is true when the instance has no users yet and requires initial
|
||||||
|
// setup (creating the first admin account). Unlike a null admin, this stays
|
||||||
|
// false once any user exists, so an instance that has lost its admins is not
|
||||||
|
// mistaken for a fresh install.
|
||||||
|
bool needs_setup = 9;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Request for instance profile.
|
// Request for instance profile.
|
||||||
|
|
|
||||||
|
|
@ -209,11 +209,17 @@ type InstanceProfile struct {
|
||||||
Demo bool `protobuf:"varint,3,opt,name=demo,proto3" json:"demo,omitempty"`
|
Demo bool `protobuf:"varint,3,opt,name=demo,proto3" json:"demo,omitempty"`
|
||||||
// Instance URL is the URL of the instance.
|
// Instance URL is the URL of the instance.
|
||||||
InstanceUrl string `protobuf:"bytes,6,opt,name=instance_url,json=instanceUrl,proto3" json:"instance_url,omitempty"`
|
InstanceUrl string `protobuf:"bytes,6,opt,name=instance_url,json=instanceUrl,proto3" json:"instance_url,omitempty"`
|
||||||
// The first administrator who set up this instance.
|
// The first administrator who set up this instance, for display purposes.
|
||||||
// When null, instance requires initial setup (creating the first admin account).
|
// May be null on an instance that has lost all admins; use needs_setup to
|
||||||
|
// determine whether initial setup is actually required.
|
||||||
Admin *User `protobuf:"bytes,7,opt,name=admin,proto3" json:"admin,omitempty"`
|
Admin *User `protobuf:"bytes,7,opt,name=admin,proto3" json:"admin,omitempty"`
|
||||||
// Commit is the current build commit of instance.
|
// Commit is the current build commit of instance.
|
||||||
Commit string `protobuf:"bytes,8,opt,name=commit,proto3" json:"commit,omitempty"`
|
Commit string `protobuf:"bytes,8,opt,name=commit,proto3" json:"commit,omitempty"`
|
||||||
|
// NeedsSetup is true when the instance has no users yet and requires initial
|
||||||
|
// setup (creating the first admin account). Unlike a null admin, this stays
|
||||||
|
// false once any user exists, so an instance that has lost its admins is not
|
||||||
|
// mistaken for a fresh install.
|
||||||
|
NeedsSetup bool `protobuf:"varint,9,opt,name=needs_setup,json=needsSetup,proto3" json:"needs_setup,omitempty"`
|
||||||
unknownFields protoimpl.UnknownFields
|
unknownFields protoimpl.UnknownFields
|
||||||
sizeCache protoimpl.SizeCache
|
sizeCache protoimpl.SizeCache
|
||||||
}
|
}
|
||||||
|
|
@ -283,6 +289,13 @@ func (x *InstanceProfile) GetCommit() string {
|
||||||
return ""
|
return ""
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func (x *InstanceProfile) GetNeedsSetup() bool {
|
||||||
|
if x != nil {
|
||||||
|
return x.NeedsSetup
|
||||||
|
}
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
|
||||||
// Request for instance profile.
|
// Request for instance profile.
|
||||||
type GetInstanceProfileRequest struct {
|
type GetInstanceProfileRequest struct {
|
||||||
state protoimpl.MessageState `protogen:"open.v1"`
|
state protoimpl.MessageState `protogen:"open.v1"`
|
||||||
|
|
@ -1780,13 +1793,15 @@ var File_api_v1_instance_service_proto protoreflect.FileDescriptor
|
||||||
|
|
||||||
const file_api_v1_instance_service_proto_rawDesc = "" +
|
const file_api_v1_instance_service_proto_rawDesc = "" +
|
||||||
"\n" +
|
"\n" +
|
||||||
"\x1dapi/v1/instance_service.proto\x12\fmemos.api.v1\x1a\x19api/v1/user_service.proto\x1a\x1cgoogle/api/annotations.proto\x1a\x17google/api/client.proto\x1a\x1fgoogle/api/field_behavior.proto\x1a\x19google/api/resource.proto\x1a\x1bgoogle/protobuf/empty.proto\x1a google/protobuf/field_mask.proto\x1a\x1fgoogle/protobuf/timestamp.proto\x1a\x17google/type/color.proto\"\xa4\x01\n" +
|
"\x1dapi/v1/instance_service.proto\x12\fmemos.api.v1\x1a\x19api/v1/user_service.proto\x1a\x1cgoogle/api/annotations.proto\x1a\x17google/api/client.proto\x1a\x1fgoogle/api/field_behavior.proto\x1a\x19google/api/resource.proto\x1a\x1bgoogle/protobuf/empty.proto\x1a google/protobuf/field_mask.proto\x1a\x1fgoogle/protobuf/timestamp.proto\x1a\x17google/type/color.proto\"\xc5\x01\n" +
|
||||||
"\x0fInstanceProfile\x12\x18\n" +
|
"\x0fInstanceProfile\x12\x18\n" +
|
||||||
"\aversion\x18\x02 \x01(\tR\aversion\x12\x12\n" +
|
"\aversion\x18\x02 \x01(\tR\aversion\x12\x12\n" +
|
||||||
"\x04demo\x18\x03 \x01(\bR\x04demo\x12!\n" +
|
"\x04demo\x18\x03 \x01(\bR\x04demo\x12!\n" +
|
||||||
"\finstance_url\x18\x06 \x01(\tR\vinstanceUrl\x12(\n" +
|
"\finstance_url\x18\x06 \x01(\tR\vinstanceUrl\x12(\n" +
|
||||||
"\x05admin\x18\a \x01(\v2\x12.memos.api.v1.UserR\x05admin\x12\x16\n" +
|
"\x05admin\x18\a \x01(\v2\x12.memos.api.v1.UserR\x05admin\x12\x16\n" +
|
||||||
"\x06commit\x18\b \x01(\tR\x06commit\"\x1b\n" +
|
"\x06commit\x18\b \x01(\tR\x06commit\x12\x1f\n" +
|
||||||
|
"\vneeds_setup\x18\t \x01(\bR\n" +
|
||||||
|
"needsSetup\"\x1b\n" +
|
||||||
"\x19GetInstanceProfileRequest\"\xcd\x1b\n" +
|
"\x19GetInstanceProfileRequest\"\xcd\x1b\n" +
|
||||||
"\x0fInstanceSetting\x12\x17\n" +
|
"\x0fInstanceSetting\x12\x17\n" +
|
||||||
"\x04name\x18\x01 \x01(\tB\x03\xe0A\bR\x04name\x12W\n" +
|
"\x04name\x18\x01 \x01(\tB\x03\xe0A\bR\x04name\x12W\n" +
|
||||||
|
|
|
||||||
|
|
@ -2702,11 +2702,19 @@ components:
|
||||||
allOf:
|
allOf:
|
||||||
- $ref: '#/components/schemas/User'
|
- $ref: '#/components/schemas/User'
|
||||||
description: |-
|
description: |-
|
||||||
The first administrator who set up this instance.
|
The first administrator who set up this instance, for display purposes.
|
||||||
When null, instance requires initial setup (creating the first admin account).
|
May be null on an instance that has lost all admins; use needs_setup to
|
||||||
|
determine whether initial setup is actually required.
|
||||||
commit:
|
commit:
|
||||||
type: string
|
type: string
|
||||||
description: Commit is the current build commit of instance.
|
description: Commit is the current build commit of instance.
|
||||||
|
needsSetup:
|
||||||
|
type: boolean
|
||||||
|
description: |-
|
||||||
|
NeedsSetup is true when the instance has no users yet and requires initial
|
||||||
|
setup (creating the first admin account). Unlike a null admin, this stays
|
||||||
|
false once any user exists, so an instance that has lost its admins is not
|
||||||
|
mistaken for a fresh install.
|
||||||
description: Instance profile message containing basic instance information.
|
description: Instance profile message containing basic instance information.
|
||||||
InstanceSetting:
|
InstanceSetting:
|
||||||
type: object
|
type: object
|
||||||
|
|
|
||||||
|
|
@ -52,12 +52,23 @@ func (s *APIV1Service) GetInstanceProfile(ctx context.Context, _ *v1pb.GetInstan
|
||||||
return nil, status.Errorf(codes.Internal, "failed to get instance admin: %v", err)
|
return nil, status.Errorf(codes.Internal, "failed to get instance admin: %v", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// needs_setup reflects whether the instance has any users at all, which is
|
||||||
|
// the real signal for first-run setup. It is deliberately independent of the
|
||||||
|
// admin lookup: an instance that has lost its admins still has users and must
|
||||||
|
// not be treated as a fresh install.
|
||||||
|
limitOne := 1
|
||||||
|
users, err := s.Store.ListUsers(ctx, &store.FindUser{Limit: &limitOne})
|
||||||
|
if err != nil {
|
||||||
|
return nil, status.Errorf(codes.Internal, "failed to list users: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
instanceProfile := &v1pb.InstanceProfile{
|
instanceProfile := &v1pb.InstanceProfile{
|
||||||
Version: s.Profile.Version,
|
Version: s.Profile.Version,
|
||||||
Demo: s.Profile.Demo,
|
Demo: s.Profile.Demo,
|
||||||
InstanceUrl: s.Profile.InstanceURL,
|
InstanceUrl: s.Profile.InstanceURL,
|
||||||
Admin: admin, // nil when not initialized
|
Admin: admin, // for display only; may be nil even on a populated instance
|
||||||
Commit: s.Profile.Commit,
|
Commit: s.Profile.Commit,
|
||||||
|
NeedsSetup: len(users) == 0,
|
||||||
}
|
}
|
||||||
return instanceProfile, nil
|
return instanceProfile, nil
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -35,8 +35,9 @@ func TestGetInstanceProfile(t *testing.T) {
|
||||||
require.True(t, resp.Demo)
|
require.True(t, resp.Demo)
|
||||||
require.Equal(t, "http://localhost:8080", resp.InstanceUrl)
|
require.Equal(t, "http://localhost:8080", resp.InstanceUrl)
|
||||||
|
|
||||||
// Instance should not be initialized since no admin users are created
|
// Instance should not be initialized since no users exist at all.
|
||||||
require.Nil(t, resp.Admin)
|
require.Nil(t, resp.Admin)
|
||||||
|
require.True(t, resp.NeedsSetup)
|
||||||
})
|
})
|
||||||
|
|
||||||
t.Run("GetInstanceProfile with initialized instance", func(t *testing.T) {
|
t.Run("GetInstanceProfile with initialized instance", func(t *testing.T) {
|
||||||
|
|
@ -66,6 +67,29 @@ func TestGetInstanceProfile(t *testing.T) {
|
||||||
// Instance should be initialized since an admin user exists
|
// Instance should be initialized since an admin user exists
|
||||||
require.NotNil(t, resp.Admin)
|
require.NotNil(t, resp.Admin)
|
||||||
require.Equal(t, hostUser.Username, resp.Admin.Username)
|
require.Equal(t, hostUser.Username, resp.Admin.Username)
|
||||||
|
require.False(t, resp.NeedsSetup)
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("GetInstanceProfile with users but no admin", func(t *testing.T) {
|
||||||
|
// Create test service for this specific test
|
||||||
|
ts := NewTestService(t)
|
||||||
|
defer ts.Cleanup()
|
||||||
|
|
||||||
|
// Create a regular user but no admin. This mirrors an instance that has
|
||||||
|
// lost all of its admins: admin is nil, but the instance is NOT a fresh
|
||||||
|
// install and must not be flagged for first-run setup.
|
||||||
|
regularUser, err := ts.CreateRegularUser(ctx, "alice")
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.NotNil(t, regularUser)
|
||||||
|
|
||||||
|
req := &v1pb.GetInstanceProfileRequest{}
|
||||||
|
resp, err := ts.Service.GetInstanceProfile(ctx, req)
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.NotNil(t, resp)
|
||||||
|
|
||||||
|
// No admin to display, but setup is already done because a user exists.
|
||||||
|
require.Nil(t, resp.Admin)
|
||||||
|
require.False(t, resp.NeedsSetup)
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -20,13 +20,15 @@ const App = () => {
|
||||||
cleanupExpiredOAuthState();
|
cleanupExpiredOAuthState();
|
||||||
}, []);
|
}, []);
|
||||||
|
|
||||||
// Redirect to sign up page if instance not initialized (no admin account exists yet).
|
// Redirect to sign up page if the instance needs initial setup (no users yet).
|
||||||
|
// needsSetup is used instead of a missing admin so an instance that has lost its
|
||||||
|
// admins isn't mistaken for a fresh install (which would create a normal user).
|
||||||
// Guard with profileLoaded so a fetch failure doesn't incorrectly trigger the redirect.
|
// Guard with profileLoaded so a fetch failure doesn't incorrectly trigger the redirect.
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (profileLoaded && !instanceProfile.admin) {
|
if (profileLoaded && instanceProfile.needsSetup) {
|
||||||
navigateTo("/auth/signup");
|
navigateTo("/auth/signup");
|
||||||
}
|
}
|
||||||
}, [profileLoaded, instanceProfile.admin, navigateTo]);
|
}, [profileLoaded, instanceProfile.needsSetup, navigateTo]);
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (instanceGeneralSetting.additionalStyle) {
|
if (instanceGeneralSetting.additionalStyle) {
|
||||||
|
|
|
||||||
|
|
@ -143,7 +143,7 @@ const SignUp = () => {
|
||||||
) : (
|
) : (
|
||||||
<p className="w-full text-2xl mt-2 text-muted-foreground">Sign up is not allowed.</p>
|
<p className="w-full text-2xl mt-2 text-muted-foreground">Sign up is not allowed.</p>
|
||||||
)}
|
)}
|
||||||
{!profile.admin ? (
|
{profile.needsSetup ? (
|
||||||
<p className="w-full mt-4 text-sm font-medium text-muted-foreground">{t("auth.host-tip")}</p>
|
<p className="w-full mt-4 text-sm font-medium text-muted-foreground">{t("auth.host-tip")}</p>
|
||||||
) : (
|
) : (
|
||||||
<p className="w-full mt-4 text-sm">
|
<p className="w-full mt-4 text-sm">
|
||||||
|
|
|
||||||
File diff suppressed because one or more lines are too long
Loading…
Reference in a new issue