Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consolidate the login code to the session store #120

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/common/models/session.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import type { ParseableUser, User } from 'src/common/models/users';
import type { SessionUser, User } from 'src/common/models/users';
import { parseBoolean } from './parsers';

export interface SessionInfo {
user: ParseableUser;
user: SessionUser;
logged_in: boolean;
message: string;
}
Expand Down
26 changes: 18 additions & 8 deletions src/common/models/users.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,26 @@ import { isNonNegInt, isValidUsername, isValidEmail }
import { Dictionary, Model } from 'src/common/models';
import { UserRole } from 'src/stores/permissions';

export interface ParseableUser {
user_id?: number;
username?: string;
// This defined the required fields of a user in the Session

export interface SessionUser {
user_id: number;
username: string;
email?: string;
first_name?: string;
last_name?: string;
is_admin?: boolean;
is_admin: boolean;
student_id?: string;
}

export const default_session_user: SessionUser = {
user_id: 0,
username: 'logged_out',
is_admin: false
};

export type ParseableUser = Partial<SessionUser>;

/**
* @class User
*/
Expand All @@ -34,15 +44,15 @@ export class User extends Model {
get all_field_names(): string[] { return User.ALL_FIELDS; }
get param_fields(): string[] { return []; }

constructor(params: ParseableUser = {}) {
constructor(params: ParseableUser = { user_id: 0, username: '', is_admin: false }) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

username: logged_out?

Perhaps export this default as a constant for use in session.ts?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good idea

super();
this.set(params);
}

set(params: ParseableUser) {
if (params.username) this.username = params.username;
if (params.user_id != undefined) this.user_id = params.user_id;
if (params.is_admin != undefined) this.is_admin = params.is_admin;
if (params.user_id) this.user_id = params.user_id;
if (params.is_admin) this.is_admin = params.is_admin;
if (params.email) this.email = params.email;
if (params.first_name) this.first_name = params.first_name;
if (params.last_name) this.last_name = params.last_name;
Expand Down Expand Up @@ -72,7 +82,7 @@ export class User extends Model {
set student_id(value: string) { this._student_id = value; }

clone() {
return new User(this.toObject() as ParseableUser);
return new User(this.toObject() as unknown as ParseableUser);
}

// The email can be the empty string or valid email address.
Expand Down
23 changes: 7 additions & 16 deletions src/components/common/Login.vue
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import { useRouter } from 'vue-router';
import { ref } from 'vue';
import { useI18n } from 'vue-i18n';
import { checkPassword } from 'src/common/api-requests/session';
import { useSessionStore } from 'src/stores/session';
import { usePermissionStore } from 'src/stores/permissions';

Expand Down Expand Up @@ -54,25 +53,17 @@ const login = async () => {
username: username.value,
password: password.value
};
const session_info = await checkPassword(username_info);

if (!session_info.logged_in || !session_info.user.user_id) {
message.value = i18n.t('authentication.failure');
} else {
// success
session.updateSessionInfo(session_info);

// permissions require access to user courses and respective roles
await session.fetchUserCourses();
await permission_store.fetchRoles();
await permission_store.fetchRoutePermissions();

const login_successful = await session.login(username_info);
if (login_successful) {
const user = session.user;
let forward = localStorage.getItem('afterLogin');
forward ||= (session_info.user.is_admin) ?
forward ||= (user.is_admin) ?
'/admin' :
`/users/${session_info.user.user_id}/courses`;
`/users/${user.user_id}/courses`;
localStorage.removeItem('afterLogin');
void router.push(forward);
} else {
message.value = i18n.t('authentication.failure');
}
};
</script>
36 changes: 28 additions & 8 deletions src/stores/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,16 @@

import { defineStore } from 'pinia';
import { api } from 'boot/axios';
import { ParseableUser, User } from 'src/common/models/users';
import type { SessionInfo } from 'src/common/models/session';
import { SessionUser, User, default_session_user } from 'src/common/models/users';
import { ParseableSessionInfo, parseSessionInfo, SessionInfo, UserPassword } from 'src/common/models/session';
import { ParseableUserCourse } from 'src/common/models/courses';
import { logger } from 'boot/logger';
import { ResponseError } from 'src/common/api-requests/errors';

import { useUserStore } from 'src/stores/users';
import { useSettingsStore } from 'src/stores/settings';
import { useProblemSetStore } from 'src/stores/problem_sets';
import { UserRole } from 'src/stores/permissions';
import { usePermissionStore, UserRole } from 'src/stores/permissions';

interface CourseInfo {
course_name: string;
Expand All @@ -25,7 +25,7 @@ interface CourseInfo {
export interface SessionState {
logged_in: boolean;
expiry: number;
user: ParseableUser;
user: SessionUser;
course: CourseInfo;
user_courses: ParseableUserCourse[];
}
Expand All @@ -36,7 +36,7 @@ export const useSessionStore = defineStore('session', {
state: (): SessionState => ({
logged_in: false,
expiry: 0,
user: { username: 'logged_out' },
user: default_session_user,
course: {
course_id: 0,
role: '',
Expand All @@ -45,7 +45,7 @@ export const useSessionStore = defineStore('session', {
user_courses: []
}),
getters: {
full_name: (state): string => `${state.user?.first_name ?? ''} ${state.user?.last_name ?? ''}`,
full_name: (state): string => `${state.user.first_name ?? ''} ${state.user.last_name ?? ''}`,
getUser: (state): User => new User(state.user),
},
actions: {
Expand All @@ -63,7 +63,7 @@ export const useSessionStore = defineStore('session', {
if (this.logged_in) {
this.user = session_info.user;
} else {
this.user = new User({ username: 'logged_out' }).toObject();
this.user = default_session_user;
}
},
setCourse(course_id: number): void {
Expand Down Expand Up @@ -102,9 +102,29 @@ export const useSessionStore = defineStore('session', {
throw response.data as ResponseError;
}
},
/**
* Attempt to login to webwork3 with username/password. If successful, fetch
* needed data (usercourses, roles, permissions).
*/
async login(user_pass: UserPassword): Promise<boolean> {
const response = await api.post('login', user_pass);
const session_info = parseSessionInfo(response.data as ParseableSessionInfo);
if (!session_info.logged_in || !session_info.user.user_id) {
return false;
} else {
// success
this.updateSessionInfo(session_info);
const permission_store = usePermissionStore();
// permissions require access to user courses and respective roles
await this.fetchUserCourses();
await permission_store.fetchRoles();
await permission_store.fetchRoutePermissions();
return true;
}
},
logout() {
this.logged_in = false;
this.user = new User({ username: 'logged_out' }).toObject();
this.user = default_session_user;
this.course = { course_id: 0, role: '', course_name: '' };
useProblemSetStore().clearAll();
useSettingsStore().clearAll();
Expand Down
33 changes: 7 additions & 26 deletions tests/stores/session.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,10 @@ import { api } from 'boot/axios';

import { getUser } from 'src/common/api-requests/user';
import { useSessionStore } from 'src/stores/session';
import { checkPassword } from 'src/common/api-requests/session';

import { Course, UserCourse } from 'src/common/models/courses';
import { SessionInfo } from 'src/common/models/session';
import { ParseableUser, User } from 'src/common/models/users';
import { SessionUser, User, default_session_user } from 'src/common/models/users';

import { cleanIDs, loadCSV } from '../utils';

Expand All @@ -30,7 +29,7 @@ describe('Session Store', () => {
let lisa: User;

// session now just stores objects not models:
const user: ParseableUser = {
const user: SessionUser = {
first_name: 'Homer',
last_name: 'Simpson',
user_id: 1234,
Expand All @@ -39,16 +38,6 @@ describe('Session Store', () => {
is_admin: false,
};

const logged_out: ParseableUser = {
username: 'logged_out',
email: '',
last_name: '',
first_name: '',
user_id: 0,
is_admin: false,
student_id: ''
};

const session_info: SessionInfo = {
logged_in: true,
user,
Expand Down Expand Up @@ -114,19 +103,12 @@ describe('Session Store', () => {
});

test('Login as a user', async () => {
// test logging in as lisa gives the proper courses.
const session_info = await checkPassword({
username: 'lisa', password: 'lisa'
});
expect(session_info.logged_in).toBe(true);
expect(session_info.user).toStrictEqual(lisa.toObject());

// test logging in as lisa gives the proper courses.
const session = useSessionStore();
session.updateSessionInfo(session_info);

const logged_in = await session.login({ username: 'lisa', password: 'lisa' });
expect(logged_in).toBe(true);
expect(session.logged_in).toBe(true);
expect(session.user).toStrictEqual(lisa.toObject());

});

// sort by course name and clean up the _id tags.
Expand All @@ -135,9 +117,8 @@ describe('Session Store', () => {
a.course_name < b.course_name ? -1 : a.course_name > b.course_name ? 1 : 0));
};

test('check user courses', async () => {
test('check user courses', () => {
const session_store = useSessionStore();
await session_store.fetchUserCourses();
expect(sortAndClean(session_store.user_courses.map(c => new UserCourse(c))))
.toStrictEqual(sortAndClean(lisa_courses));
});
Expand All @@ -164,7 +145,7 @@ describe('Session Store', () => {
session.logout();

expect(session.logged_in).toBe(false);
expect(session.user).toStrictEqual(logged_out);
expect(session.user).toStrictEqual(default_session_user);

});
});
Expand Down
1 change: 0 additions & 1 deletion tests/stores/set_problems.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import { Dictionary, generic } from 'src/common/models';

import { loadCSV, cleanIDs } from '../utils';
import { checkPassword } from 'src/common/api-requests/session';
import { logger } from 'src/boot/logger';

const app = createApp({});

Expand Down
1 change: 0 additions & 1 deletion tests/stores/user_sets.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import { createApp } from 'vue';
import { createPinia, setActivePinia } from 'pinia';
import piniaPluginPersistedstate from 'pinia-plugin-persistedstate';
import { api } from 'boot/axios';

import { useCourseStore } from 'src/stores/courses';
import { useProblemSetStore } from 'src/stores/problem_sets';
Expand Down
6 changes: 3 additions & 3 deletions tests/unit-tests/users.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,23 +96,23 @@ describe('Testing User and CourseUsers', () => {
describe('Testing for valid and invalid users.', () => {

test('setting invalid email', () => {
const user = new User({ username: 'test' });
const user = new User({ username: 'test', user_id: 10, is_admin: true });
expect(user.isValid()).toBe(true);

user.email = 'bad@[email protected]';
expect(user.isValid()).toBe(false);
});

test('setting invalid user_id', () => {
const user = new User({ username: 'test' });
const user = new User({ username: 'test', user_id: 10, is_admin: true });
expect(user.isValid()).toBe(true);

user.user_id = -15;
expect(user.isValid()).toBe(false);
});

test('setting invalid username', () => {
const user = new User({ username: 'my username' });
const user = new User({ username: 'my username', user_id: 10, is_admin: true });
expect(user.isValid()).toBe(false);
});
});
Expand Down