vbatts/homebox
1
0
Fork 0
forked from mirrors/homebox
Code Pull requests Activity

feat: change password (#35)

Browse source 
* refactor: implement factories for testing

* add additional factories

* change protection for dropFields

* prevent timed attacks on login

* use switch instead of else-if

* API implementation for changing password

* add change-password dialog
This commit is contained in:
Hayden 2022-10-09 09:23:21 -08:00 committed by GitHub
parent a6e3989aee
commit a6d2fd45df
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
19 changed files with 458 additions and 149 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
Taskfile.yml
View file

@ -20,7 +20,7 @@ tasks:
--path ./backend/app/api/docs/swagger.json \
--output ./frontend/lib/api/types
python3 ./scripts/process-types.py ./frontend/lib/api/types/data-contracts.ts
- python3 ./scripts/process-types.py ./frontend/lib/api/types/data-contracts.ts
sources:
- "./backend/app/api/**/*"
- "./backend/internal/repo/**/*"

40
backend/app/api/docs/docs.go
View file

@ -822,6 +822,35 @@ const docTemplate = `{
}
}
},
"/v1/users/change-password": {
"put": {
"security": [
{
"Bearer": []
}
],
"tags": [
"User"
],
"summary": "Updates the users password",
"parameters": [
{
"description": "Password Payload",
"name": "payload",
"in": "body",
"required": true,
"schema": {
"$ref": "#/definitions/v1.ChangePassword"
}
}
],
"responses": {
"204": {
"description": ""
}
}
}
},
"/v1/users/login": {
"post": {
"consumes": [
@ -1579,6 +1608,17 @@ const docTemplate = `{
}
}
},
"v1.ChangePassword": {
"type": "object",
"properties": {
"current": {
"type": "string"
},
"new": {
"type": "string"
}
}
},
"v1.GroupInvitation": {
"type": "object",
"properties": {

40
backend/app/api/docs/swagger.json
View file

@ -814,6 +814,35 @@
}
}
},
"/v1/users/change-password": {
"put": {
"security": [
{
"Bearer": []
}
],
"tags": [
"User"
],
"summary": "Updates the users password",
"parameters": [
{
"description": "Password Payload",
"name": "payload",
"in": "body",
"required": true,
"schema": {
"$ref": "#/definitions/v1.ChangePassword"
}
}
],
"responses": {
"204": {
"description": ""
}
}
}
},
"/v1/users/login": {
"post": {
"consumes": [
@ -1571,6 +1600,17 @@
}
}
},
"v1.ChangePassword": {
"type": "object",
"properties": {
"current": {
"type": "string"
},
"new": {
"type": "string"
}
}
},
"v1.GroupInvitation": {
"type": "object",
"properties": {

24
backend/app/api/docs/swagger.yaml
View file

@ -353,6 +353,13 @@ definitions:
version:
type: string
type: object
v1.ChangePassword:
properties:
current:
type: string
new:
type: string
type: object
v1.GroupInvitation:
properties:
expiresAt:
@ -877,6 +884,23 @@ paths:
summary: Retrieves the basic information about the API
tags:
- Base
/v1/users/change-password:
put:
parameters:
- description: Password Payload
in: body
name: payload
required: true
schema:
$ref: '#/definitions/v1.ChangePassword'
responses:
"204":
description: ""
security:
- Bearer: []
summary: Updates the users password
tags:
- User
/v1/users/login:
post:
consumes:

1
backend/app/api/routes.go
View file

@ -65,6 +65,7 @@ func (a *app) newRouter(repos *repo.AllRepos) *chi.Mux {
r.Put(v1Base("/users/self/password"), v1Ctrl.HandleUserUpdatePassword())
r.Post(v1Base("/users/logout"), v1Ctrl.HandleAuthLogout())
r.Get(v1Base("/users/refresh"), v1Ctrl.HandleAuthRefresh())
r.Put(v1Base("/users/self/change-password"), v1Ctrl.HandleUserSelfChangePassword())
r.Post(v1Base("/groups/invitations"), v1Ctrl.HandleGroupInvitationsCreate())

9
backend/app/api/v1/v1_ctrl_auth.go
View file

@ -36,7 +36,8 @@ func (ctrl *V1Controller) HandleAuthLogin() http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
loginForm := &LoginForm{}
if r.Header.Get("Content-Type") == server.ContentFormUrlEncoded {
switch r.Header.Get("Content-Type") {
case server.ContentFormUrlEncoded:
err := r.ParseForm()
if err != nil {
server.Respond(w, http.StatusBadRequest, server.Wrap(err))
@ -46,7 +47,7 @@ func (ctrl *V1Controller) HandleAuthLogin() http.HandlerFunc {
loginForm.Username = r.PostFormValue("username")
loginForm.Password = r.PostFormValue("password")
} else if r.Header.Get("Content-Type") == server.ContentJSON {
case server.ContentJSON:
err := server.Decode(r, loginForm)
if err != nil {
@ -54,7 +55,7 @@ func (ctrl *V1Controller) HandleAuthLogin() http.HandlerFunc {
server.Respond(w, http.StatusBadRequest, server.Wrap(err))
return
}
} else {
default:
server.Respond(w, http.StatusBadRequest, errors.New("invalid content type"))
return
}
@ -67,7 +68,7 @@ func (ctrl *V1Controller) HandleAuthLogin() http.HandlerFunc {
newToken, err := ctrl.svc.User.Login(r.Context(), loginForm.Username, loginForm.Password)
if err != nil {
server.RespondError(w, http.StatusUnauthorized, err)
server.RespondError(w, http.StatusInternalServerError, err)
return
}

34
backend/app/api/v1/v1_ctrl_user.go
View file

@ -119,3 +119,37 @@ func (ctrl *V1Controller) HandleUserSelfDelete() http.HandlerFunc {
server.Respond(w, http.StatusNoContent, nil)
}
}
type (
ChangePassword struct {
Current string `json:"current,omitempty"`
New string `json:"new,omitempty"`
}
)
// HandleUserSelfChangePassword godoc
// @Summary Updates the users password
// @Tags User
// @Success 204
// @Param payload body ChangePassword true "Password Payload"
// @Router /v1/users/change-password [PUT]
// @Security Bearer
func (ctrl *V1Controller) HandleUserSelfChangePassword() http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
var cp ChangePassword
err := server.Decode(r, &cp)
if err != nil {
log.Err(err).Msg("user failed to change password")
}
ctx := services.NewContext(r.Context())
ok := ctrl.svc.User.ChangePassword(ctx, cp.Current, cp.New)
if !ok {
server.RespondError(w, http.StatusInternalServerError, err)
return
}
server.Respond(w, http.StatusNoContent, nil)
}
}

4
backend/internal/repo/repo_users.go
View file

@ -121,3 +121,7 @@ func (e *UserRepository) GetSuperusers(ctx context.Context) ([]*ent.User, error)
return users, nil
}
func (r *UserRepository) ChangePassword(ctx context.Context, UID uuid.UUID, pw string) error {
return r.db.User.UpdateOneID(UID).SetPassword(pw).Exec(ctx)
}

34
backend/internal/services/service_user.go
View file

@ -142,7 +142,13 @@ func (svc *UserService) createToken(ctx context.Context, userId uuid.UUID) (User
func (svc *UserService) Login(ctx context.Context, username, password string) (UserAuthTokenDetail, error) {
usr, err := svc.repos.Users.GetOneEmail(ctx, username)
if err != nil || !hasher.CheckPasswordHash(password, usr.PasswordHash) {
if err != nil {
// SECURITY: Perform hash to ensure response times are the same
hasher.CheckPasswordHash("not-a-real-password", "not-a-real-password")
return UserAuthTokenDetail{}, ErrorInvalidLogin
}
if !hasher.CheckPasswordHash(password, usr.PasswordHash) {
return UserAuthTokenDetail{}, ErrorInvalidLogin
}
@ -190,3 +196,29 @@ func (svc *UserService) NewInvitation(ctx Context, uses int, expiresAt time.Time
return token.Raw, nil
}
func (svc *UserService) ChangePassword(ctx Context, current string, new string) (ok bool) {
usr, err := svc.repos.Users.GetOneId(ctx, ctx.UID)
if err != nil {
return false
}
if !hasher.CheckPasswordHash(current, usr.PasswordHash) {
log.Err(errors.New("current password is incorrect")).Msg("Failed to change password")
return false
}
hashed, err := hasher.HashPassword(new)
if err != nil {
log.Err(err).Msg("Failed to hash password")
return false
}
err = svc.repos.Users.ChangePassword(ctx.Context, ctx.UID, hashed)
if err != nil {
log.Err(err).Msg("Failed to change password")
return false
}
return true
}

80
frontend/lib/api/__test__/factories/index.ts Normal file
View file

@ -0,0 +1,80 @@
import { faker } from "@faker-js/faker";
import { expect } from "vitest";
import { overrideParts } from "../../base/urls";
import { PublicApi } from "../../public";
import { LabelCreate, LocationCreate, UserRegistration } from "../../types/data-contracts";
import * as config from "../../../../test/config";
import { UserClient } from "../../user";
import { Requests } from "../../../requests";
/**
* Returns a random user registration object that can be
* used to signup a new user.
*/
function user(): UserRegistration {
return {
email: faker.internet.email(),
password: faker.internet.password(),
name: faker.name.firstName(),
token: "",
};
}
function location(): LocationCreate {
return {
name: faker.address.city(),
description: faker.lorem.sentence(),
};
}
function label(): LabelCreate {
return {
name: faker.lorem.word(),
description: faker.lorem.sentence(),
color: faker.internet.color(),
};
}
function publicClient(): PublicApi {
overrideParts(config.BASE_URL, "/api/v1");
const requests = new Requests("");
return new PublicApi(requests);
}
function userClient(token: string): UserClient {
overrideParts(config.BASE_URL, "/api/v1");
const requests = new Requests("", token);
return new UserClient(requests);
}
type TestUser = {
client: UserClient;
user: UserRegistration;
};
async function userSingleUse(): Promise<TestUser> {
const usr = user();
const pub = publicClient();
await pub.register(usr);
const result = await pub.login(usr.email, usr.password);
expect(result.error).toBeFalsy();
expect(result.status).toBe(200);
return {
client: new UserClient(new Requests("", result.data.token)),
user: usr,
};
}
export const factories = {
user,
location,
label,
client: {
public: publicClient,
user: userClient,
singleUse: userSingleUse,
},
};

26
frontend/lib/api/__test__/public.test.ts
View file

@ -1,20 +1,10 @@
import { describe, test, expect } from "vitest";
import { faker } from "@faker-js/faker";
import { UserRegistration } from "../types/data-contracts";
import { client, sharedUserClient, userClient } from "./test-utils";
function userFactory(): UserRegistration {
return {
email: faker.internet.email(),
password: faker.internet.password(),
name: faker.name.firstName(),
token: "",
};
}
import { factories } from "./factories";
import { sharedUserClient } from "./test-utils";
describe("[GET] /api/v1/status", () => {
test("server should respond", async () => {
const api = client();
const api = factories.client.public();
const { response, data } = await api.status();
expect(response.status).toBe(200);
expect(data.health).toBe(true);
@ -22,8 +12,8 @@ describe("[GET] /api/v1/status", () => {
});
describe("first time user workflow (register, login, join group)", () => {
const api = client();
const userData = userFactory();
const api = factories.client.public();
const userData = factories.user();
test("user should be able to register", async () => {
const { response } = await api.register(userData);
@ -36,7 +26,7 @@ describe("first time user workflow (register, login, join group)", () => {
expect(data.token).toBeTruthy();
// Cleanup
const userApi = userClient(data.token);
const userApi = factories.client.user(data.token);
{
const { response } = await userApi.user.delete();
expect(response.status).toBe(204);
@ -59,7 +49,7 @@ describe("first time user workflow (register, login, join group)", () => {
// Create User 2 with token
const duplicateUser = userFactory();
const duplicateUser = factories.user();
duplicateUser.token = data.token;
const { response: registerResp } = await api.register(duplicateUser);
@ -70,7 +60,7 @@ describe("first time user workflow (register, login, join group)", () => {
// Get Self and Assert
const client2 = userClient(loginData.token);
const client2 = factories.client.user(loginData.token);
const { data: user2 } = await client2.user.self();

25
frontend/lib/api/__test__/test-utils.ts
View file

@ -1,21 +1,6 @@
import { beforeAll, expect } from "vitest";
import { Requests } from "../../requests";
import { overrideParts } from "../base/urls";
import { PublicApi } from "../public";
import * as config from "../../../test/config";
import { UserClient } from "../user";
export function client() {
overrideParts(config.BASE_URL, "/api/v1");
const requests = new Requests("");
return new PublicApi(requests);
}
export function userClient(token: string) {
overrideParts(config.BASE_URL, "/api/v1");
const requests = new Requests("", token);
return new UserClient(requests);
}
import { factories } from "./factories";
const cache = {
token: "",
@ -27,7 +12,7 @@ const cache = {
*/
export async function sharedUserClient(): Promise<UserClient> {
if (cache.token) {
return userClient(cache.token);
return factories.client.user(cache.token);
}
const testUser = {
email: "__test__@__test__.com",
@ -36,12 +21,12 @@ export async function sharedUserClient(): Promise<UserClient> {
token: "",
};
const api = client();
const api = factories.client.public();
const { response: tryLoginResp, data } = await api.login(testUser.email, testUser.password);
if (tryLoginResp.status === 200) {
cache.token = data.token;
return userClient(cache.token);
return factories.client.user(cache.token);
}
const { response: registerResp } = await api.register(testUser);
@ -51,7 +36,7 @@ export async function sharedUserClient(): Promise<UserClient> {
expect(loginResp.status).toBe(200);
cache.token = loginData.token;
return userClient(data.token);
return factories.client.user(data.token);
}
beforeAll(async () => {

16
frontend/lib/api/__test__/user/labels.test.ts
View file

@ -1,23 +1,17 @@
import { describe, expect, test } from "vitest";
import { LabelOut } from "../../types/data-contracts";
import { UserClient } from "../../user";
import { factories } from "../factories";
import { sharedUserClient } from "../test-utils";
describe("locations lifecycle (create, update, delete)", () => {
let increment = 0;
/**
* useLabel sets up a label resource for testing, and returns a function
* that can be used to delete the label from the backend server.
*/
async function useLabel(api: UserClient): Promise<[LabelOut, () => Promise<void>]> {
const { response, data } = await api.labels.create({
name: `__test__.label.name_${increment}`,
description: `__test__.label.description_${increment}`,
color: "",
});
const { response, data } = await api.labels.create(factories.label());
expect(response.status).toBe(201);
increment++;
const cleanup = async () => {
const { response } = await api.labels.delete(data.id);
@ -29,11 +23,7 @@ describe("locations lifecycle (create, update, delete)", () => {
test("user should be able to create a label", async () => {
const api = await sharedUserClient();
const labelData = {
name: "test-label",
description: "test-description",
color: "",
};
const labelData = factories.label();
const { response, data } = await api.labels.create(labelData);

14
frontend/lib/api/__test__/user/locations.test.ts
View file

@ -1,22 +1,17 @@
import { describe, expect, test } from "vitest";
import { LocationOut } from "../../types/data-contracts";
import { UserClient } from "../../user";
import { factories } from "../factories";
import { sharedUserClient } from "../test-utils";
describe("locations lifecycle (create, update, delete)", () => {
let increment = 0;
/**
* useLocatio sets up a location resource for testing, and returns a function
* that can be used to delete the location from the backend server.
*/
async function useLocation(api: UserClient): Promise<[LocationOut, () => Promise<void>]> {
const { response, data } = await api.locations.create({
name: `__test__.location.name_${increment}`,
description: `__test__.location.description_${increment}`,
});
const { response, data } = await api.locations.create(factories.location());
expect(response.status).toBe(201);
increment++;
const cleanup = async () => {
const { response } = await api.locations.delete(data.id);
@ -29,10 +24,7 @@ describe("locations lifecycle (create, update, delete)", () => {
test("user should be able to create a location", async () => {
const api = await sharedUserClient();
const locationData = {
name: "test-location",
description: "test-description",
};
const locationData = factories.location();
const { response, data } = await api.locations.create(locationData);

27
frontend/lib/api/__test__/user/user.test.ts Normal file
View file

@ -0,0 +1,27 @@
import { faker } from "@faker-js/faker";
import { describe, expect, test } from "vitest";
import { factories } from "../factories";
describe("basic user workflows", () => {
test("user should be able to change password", async () => {
const { client, user } = await factories.client.singleUse();
const password = faker.internet.password();
// Change Password
{
const response = await client.user.changePassword(user.password, password);
expect(response.error).toBeFalsy();
expect(response.status).toBe(204);
}
// Ensure New Login is Valid
{
const pub = factories.client.public();
const response = await pub.login(user.email, password);
expect(response.error).toBeFalsy();
expect(response.status).toBe(200);
}
await client.user.delete();
}, 20000);
});

2
frontend/lib/api/base/base-api.ts
View file

@ -43,7 +43,7 @@ export class BaseAPI {
* are present. This is useful for when you want to send a subset of fields to
* the server like when performing an update.
*/
dropFields<T>(obj: T, keys: Array<keyof T> = []): T {
protected dropFields<T>(obj: T, keys: Array<keyof T> = []): T {
const result = { ...obj };
[...keys, "createdAt", "updatedAt"].forEach(key => {
// @ts-ignore - we are checking for the key above

12
frontend/lib/api/classes/users.ts
View file

@ -1,5 +1,5 @@
import { BaseAPI, route } from "../base";
import { UserOut } from "../types/data-contracts";
import { ChangePassword, UserOut } from "../types/data-contracts";
import { Result } from "../types/non-generated";
export class UserApi extends BaseAPI {
@ -14,4 +14,14 @@ export class UserApi extends BaseAPI {
public delete() {
return this.http.delete<void>({ url: route("/users/self") });
}
public changePassword(current: string, newPassword: string) {
return this.http.put<ChangePassword, void>({
url: route("/users/self/change-password"),
body: {
current,
new: newPassword,
},
});
}
}

5
frontend/lib/api/types/data-contracts.ts
View file

@ -238,6 +238,11 @@ export interface Build {
version: string;
}
export interface ChangePassword {
current: string;
new: string;
}
export interface GroupInvitation {
expiresAt: Date;
token: string;

56
frontend/pages/profile.vue
View file

@ -192,9 +192,62 @@
token.value = data.token;
}
}
const passwordChange = reactive({
loading: false,
dialog: false,
current: "",
new: "",
isValid: false,
});
function openPassChange() {
passwordChange.dialog = true;
}
async function changePassword() {
passwordChange.loading = true;
if (!passwordChange.isValid) {
return;
}
const { error } = await api.user.changePassword(passwordChange.current, passwordChange.new);
if (error) {
notify.error("Failed to change password.");
passwordChange.loading = false;
return;
}
notify.success("Password changed successfully.");
passwordChange.dialog = false;
passwordChange.new = "";
passwordChange.current = "";
passwordChange.loading = false;
}
</script>
<template>
<div>
<BaseModal v-model="passwordChange.dialog">
<template #title> Change Password </template>
<FormTextField v-model="passwordChange.current" label="Current Password" type="password" />
<FormTextField v-model="passwordChange.new" label="New Password" type="password" />
<PasswordScore v-model:valid="passwordChange.isValid" :password="passwordChange.new" />
<div class="flex">
<BaseButton
class="ml-auto"
:loading="passwordChange.loading"
:disabled="!passwordChange.isValid"
@click="changePassword"
>
Submit
</BaseButton>
</div>
</BaseModal>
<BaseContainer class="flex flex-col gap-4 mb-6">
<BaseCard>
<template #title>
@ -209,7 +262,7 @@
<div class="p-4">
<div class="flex gap-2">
<BaseButton size="sm"> Change Password </BaseButton>
<BaseButton size="sm" @click="openPassChange"> Change Password </BaseButton>
<BaseButton size="sm" @click="generateToken"> Generate Invite Link </BaseButton>
</div>
<div v-if="token" class="pt-4 flex items-center pl-1">
@ -288,6 +341,7 @@
</template>
</BaseCard>
</BaseContainer>
</div>
</template>
<style scoped></style>