diff --git a/packages/api-client/index.ts b/packages/api-client/index.ts index 77b698661..5745b8b03 100644 --- a/packages/api-client/index.ts +++ b/packages/api-client/index.ts @@ -3,7 +3,6 @@ import { CreateAlertResponse, CreateQuestionParams, CreateQuestionResponse, - DateRangeType, DesktopNotifBody, DesktopNotifPartial, GetAlertsResponse, @@ -15,10 +14,7 @@ import { GetQueueResponse, GetReleaseNotesResponse, GetSelfEnrollResponse, - ListInsightsResponse, ListQuestionsResponse, - SemesterPartial, - SubmitCourseParams, TACheckinTimesResponse, TACheckoutResponse, TAUpdateStatusResponse, @@ -28,6 +24,14 @@ import { UpdateQuestionParams, UpdateQuestionResponse, UpdateQueueParams, + ListInsightsResponse, + DateRangeType, + SubmitCourseParams, + SemesterPartial, + OAuthAccessTokensResponse, + OAuthAccessTokensRequest, + AccessToken, + RefreshToken, } from "@koh/common"; import Axios, { AxiosInstance, Method } from "axios"; import { plainToClass } from "class-transformer"; @@ -235,7 +239,21 @@ class APIClient { this.req("PATCH", `/api/v1/alerts/${alertId}`); }, }; - + oauth = { + tokens: async ( + param: OAuthAccessTokensRequest + ): Promise<OAuthAccessTokensResponse> => + this.req( + "POST", + `/api/v1/oauth/tokens`, + OAuthAccessTokensResponse, + param + ), + renewToken: async (param: RefreshToken): Promise<AccessToken> => + this.req("POST", "/api/v1/oauth/tokens/refresh", AccessToken, param), + userInfo: async (param: AccessToken): Promise<void> => + this.req("POST", `/api/v1/oauth/user`, undefined, param), + }; constructor(baseURL = "") { this.axios = Axios.create({ baseURL: baseURL }); } diff --git a/packages/app/components/OAuth/OAuthErrorPage.tsx b/packages/app/components/OAuth/OAuthErrorPage.tsx new file mode 100644 index 000000000..c29db5ba2 --- /dev/null +++ b/packages/app/components/OAuth/OAuthErrorPage.tsx @@ -0,0 +1,32 @@ +import React, { ReactElement } from "react"; +import styled from "styled-components"; +import { Button } from "antd"; +import Router from "next/router"; + +const Container = styled.div` + height: 80vh; + display: flex; + justify-content: center; + align-items: center; +`; + +const ContentContainer = styled.div` + text-align: center; +`; + +export default function OAuthErrorPage(): ReactElement { + return ( + <Container> + <ContentContainer> + <h3> An error occurred while trying to login. Please try again. </h3> + <Button + onClick={() => { + Router.push("/login"); + }} + > + Back Home + </Button> + </ContentContainer> + </Container> + ); +} diff --git a/packages/app/components/Settings/SettingsPage.tsx b/packages/app/components/Settings/SettingsPage.tsx index ece9249bb..f3bb0b50d 100644 --- a/packages/app/components/Settings/SettingsPage.tsx +++ b/packages/app/components/Settings/SettingsPage.tsx @@ -37,11 +37,9 @@ const ProfilePicButton = styled(Button)` export default function SettingsPage({ defaultPage, }: SettingsPageProps): ReactElement { - const { - data: profile, - error, - mutate, - } = useSWR(`api/v1/profile`, async () => API.profile.index()); + const { data: profile, error, mutate } = useSWR(`api/v1/profile`, async () => + API.profile.index() + ); const [currentSettings, setCurrentSettings] = useState( defaultPage || SettingsOptions.PROFILE diff --git a/packages/app/components/Today/TACheckinButton.tsx b/packages/app/components/Today/TACheckinButton.tsx index de360105c..e83930411 100644 --- a/packages/app/components/Today/TACheckinButton.tsx +++ b/packages/app/components/Today/TACheckinButton.tsx @@ -58,11 +58,10 @@ export default function TACheckinButton({ ); } - const [checkoutModalInfo, setCheckoutModalInfo] = - useState<{ - canClearQueue: boolean; - nextOfficeHourTime?: Date; - }>(EMPTY_CHECKOUT_INFO); + const [checkoutModalInfo, setCheckoutModalInfo] = useState<{ + canClearQueue: boolean; + nextOfficeHourTime?: Date; + }>(EMPTY_CHECKOUT_INFO); const closeModal = () => setCheckoutModalInfo(EMPTY_CHECKOUT_INFO); return ( diff --git a/packages/app/hooks/useDefaultCourseRedirect.ts b/packages/app/hooks/useDefaultCourseRedirect.ts index 2e850b62d..a5c73e51b 100644 --- a/packages/app/hooks/useDefaultCourseRedirect.ts +++ b/packages/app/hooks/useDefaultCourseRedirect.ts @@ -8,6 +8,7 @@ export function useDefaultCourseRedirect(): boolean { const [defaultCourse] = useLocalStorage("defaultCourse", null); if (profile && profile.courses.length > 0) { /// defaultCourse can get out-of-sync with the user's actual registered course (dropped class etc) + // TODO: Change from !!defaultCourse to defaultCourse const isUserInDefaultCourse = !!defaultCourse && profile.courses.some((c) => c.course.id === defaultCourse?.id); diff --git a/packages/app/hooks/useDraftQuestion.ts b/packages/app/hooks/useDraftQuestion.ts index 9598bb736..72fa3c4de 100644 --- a/packages/app/hooks/useDraftQuestion.ts +++ b/packages/app/hooks/useDraftQuestion.ts @@ -7,8 +7,11 @@ interface UseDraftQuestionResult { deleteDraftQuestion: () => void; } export function useDraftQuestion(): UseDraftQuestionResult { - const [draftQuestion, setDraftQuestion, deleteDraftQuestion] = - useLocalStorage("draftQuestion", null); + const [ + draftQuestion, + setDraftQuestion, + deleteDraftQuestion, + ] = useLocalStorage("draftQuestion", null); return { draftQuestion, setDraftQuestion, diff --git a/packages/app/hooks/useQueue.ts b/packages/app/hooks/useQueue.ts index ed4b2b952..666ca06d1 100644 --- a/packages/app/hooks/useQueue.ts +++ b/packages/app/hooks/useQueue.ts @@ -72,11 +72,7 @@ export function useQueue(qid: number, onUpdate?: OnUpdate): UseQueueReturn { ) ); - const { - data: queue, - error: queueError, - mutate: mutateQueue, - } = useSWR( + const { data: queue, error: queueError, mutate: mutateQueue } = useSWR( key, useCallback(async () => API.queues.get(Number(qid)), [qid]), { diff --git a/packages/app/hooks/useSaveStateChallenge.tsx b/packages/app/hooks/useSaveStateChallenge.tsx new file mode 100644 index 000000000..b97d74dfb --- /dev/null +++ b/packages/app/hooks/useSaveStateChallenge.tsx @@ -0,0 +1,23 @@ +export interface OAuthStateChallenge { + state: string; + challenge: string; +} + +/** + * + * @param stateLength How long the state value should be + * @param hashLength How long the plaintext value of the challenge should be + * @returns The pair of the state and challenge plaintext value + */ +export function useSaveStateChallenge( + stateLength: number, + hashLength: number +): OAuthStateChallenge { + const state = Math.random().toString(20).substr(2, stateLength); + const challenge = Math.random().toString(20).substr(2, hashLength); + + return { + state, + challenge, + }; +} diff --git a/packages/app/package.json b/packages/app/package.json index 18de0564e..efddcc466 100644 --- a/packages/app/package.json +++ b/packages/app/package.json @@ -35,6 +35,7 @@ "next-compose-plugins": "^2.2.0", "next-offline": "^5.0.2", "next-transpile-modules": "^3.3.0", + "node-forge": "^0.10.0", "platform": "^1.3.6", "react": "16.13.1", "react-big-calendar": "^0.24.6", diff --git a/packages/app/pages/_app.tsx b/packages/app/pages/_app.tsx index 08d22b12f..1526d7633 100644 --- a/packages/app/pages/_app.tsx +++ b/packages/app/pages/_app.tsx @@ -11,7 +11,8 @@ import * as Sentry from "@sentry/node"; if (process.env.NODE_ENV === "production" && typeof window !== "undefined") { Sentry.init({ enabled: process.env.NODE_ENV === "production", - dsn: "https://9cfb47804c93495ba3a66a9d79cec084@o440615.ingest.sentry.io/5557379", + dsn: + "https://9cfb47804c93495ba3a66a9d79cec084@o440615.ingest.sentry.io/5557379", tracesSampleRate: 0.2, }); } diff --git a/packages/app/pages/login.tsx b/packages/app/pages/login.tsx index 7b576264b..c506f9b3a 100644 --- a/packages/app/pages/login.tsx +++ b/packages/app/pages/login.tsx @@ -5,6 +5,16 @@ import { useDefaultCourseRedirect } from "../hooks/useDefaultCourseRedirect"; import { User } from "@koh/common"; import Router from "next/router"; import { useProfile } from "../hooks/useProfile"; +import { useSaveStateChallenge } from "../hooks/useSaveStateChallenge"; +import { + KHOURY_ADMIN_OAUTH_URL, + OAUTH_CLIENT_ID, + OAUTH_REDIRECT_URI, + OAUTH_SCOPES, +} from "@koh/common"; + +let forge = require("node-forge"); +const isWindow = typeof window !== "undefined"; const Container = styled.div` height: 80vh; @@ -16,6 +26,38 @@ const Container = styled.div` const ContentContainer = styled.div` text-align: center; `; +/** + * Opens a new window that directs a user to the Khoury OAuth login page and passes in + * the Office Hour client properties used for verification. + */ +function openKhouryOAuthLoginPage() { + let stateVal = ""; + let codeVal = ""; + if (isWindow) { + stateVal = window.localStorage.getItem("state"); + codeVal = window.localStorage.getItem("challenge"); + } + let md = forge.md.sha256.create(); + md.update(codeVal); + const hashedCodeChallenge: string = md.digest().toHex(); + const windowReference = window.open( + KHOURY_ADMIN_OAUTH_URL + + "/login?response_type=code&client_id=" + + OAUTH_CLIENT_ID + + "&redirect_uri=" + + OAUTH_REDIRECT_URI + + "&" + + OAUTH_SCOPES + + "&state=" + + stateVal + + "&challenge=" + + hashedCodeChallenge, + "_blank" + ); + if (window.focus) { + windowReference.focus(); + } +} export default function Login(): ReactElement { const profile: User = useProfile(); @@ -24,12 +66,28 @@ export default function Login(): ReactElement { Router.push("/nocourses"); } + const stateChallengeLocalStorage = useSaveStateChallenge(6, 6); + const localState = stateChallengeLocalStorage.state; + const localChallenge = stateChallengeLocalStorage.challenge; + + if ( + isWindow && + window.localStorage.getItem("state") === null && + window.localStorage.getItem("challenge") === null + ) { + window.localStorage.setItem("state", localState); + window.localStorage.setItem("challenge", localChallenge); + } return ( <Container> <ContentContainer> <h1>You are currently not logged in</h1> <p>Click the button below to login via Khoury Admin</p> - <Button href="https://admin.khoury.northeastern.edu/teaching/officehourslogin/"> + <Button + onClick={() => { + openKhouryOAuthLoginPage(); + }} + > Log in via Khoury Admin </Button> </ContentContainer> diff --git a/packages/app/pages/oauth.tsx b/packages/app/pages/oauth.tsx new file mode 100644 index 000000000..547a64214 --- /dev/null +++ b/packages/app/pages/oauth.tsx @@ -0,0 +1,72 @@ +import { API } from "@koh/api-client"; +import { useRouter } from "next/router"; +import { ReactElement, useState } from "react"; +import { StandardPageContainer } from "../components/common/PageContainer"; +import Router from "next/router"; +import { OAuthAccessTokensRequest } from "@koh/common"; +import { Spin } from "antd"; +import OAuthErrorPage from "../components/OAuth/OAuthErrorPage"; + +const isWindow = typeof window !== "undefined"; + +async function signUserIn(request: OAuthAccessTokensRequest): Promise<boolean> { + let tokens; + try { + tokens = await API.oauth.tokens(request); + } catch (err) { + return false; + } + let params = { + access: tokens.access, + }; + const userLoginResult = await API.oauth + .userInfo(params) + .then(() => { + return true; + }) + .catch(() => { + return false; + }); + return userLoginResult; +} + +export default function OAuth(): ReactElement { + const router = useRouter(); + const state = router.query.state; + const authCode = router.query.code; + let [hasError, setHasError] = useState(false); + + let tokensRequestBody: OAuthAccessTokensRequest; + + if (state && authCode && isWindow) { + const storedState = window.localStorage.getItem("state"); + const storedChallenge = window.localStorage.getItem("challenge"); + if (storedState != state) { + // if the states are not equal then a CRSF attack may be happening so do not continue with OAuth sign in and return to login + // also, may want to notify khoury admin page. An error also could have happened so that is another possibility. + Router.push("/login"); + } else { + tokensRequestBody = { + code: authCode as string, + verifier: storedChallenge, + }; + signUserIn(tokensRequestBody) + .then((result) => { + if (result) { + Router.push("/nocourses"); + } else { + setHasError(true); + } + }) + .catch(() => { + setHasError(true); + }); + } + } + + return ( + <StandardPageContainer> + {hasError ? <OAuthErrorPage /> : <Spin style={{ margin: "10% 45%" }} />} + </StandardPageContainer> + ); +} diff --git a/packages/common/index.ts b/packages/common/index.ts index abb5998b5..541e798e5 100644 --- a/packages/common/index.ts +++ b/packages/common/index.ts @@ -17,6 +17,17 @@ import "reflect-metadata"; export const PROD_URL = "https://officehours.khoury.northeastern.edu"; export const STAGING_URL = "https://staging.khouryofficehours.com"; + +export const KHOURY_ADMIN_OAUTH_API_URL = + "https://admin.khoury.northeastern.edu/api/oauth"; +export const KHOURY_ADMIN_OAUTH_URL = + "https://admin.khoury.northeastern.edu/oauth"; +export const OAUTH_CLIENT_ID = "<replace_with_khoury_client_id>"; +export const OAUTH_CLIENT_SECRET = "<replace_with_khoury_client_secret>"; +export const OAUTH_REDIRECT_URI = "<replace_with_sandbox_oauth_page>"; +export const OAUTH_SCOPES = + "scopes=user.info&scopes=ta.info&scopes=student.courses&scopes=instructor.courses"; + // Get domain. works on node and browser const domain = (): string | false => process.env.DOMAIN || @@ -327,6 +338,46 @@ export type PhoneNotifBody = { // Office Hours Response Types export class GetProfileResponse extends User {} +/** + * @param access - represents an access token that can be used to get data from the Khoury Admin OAuth routes + * @param refresh - represents a refresh token that can be used to get new access tokens once the access token expires + */ +export class OAuthAccessTokensResponse { + @IsString() + access!: string; + + @IsString() + refresh!: string; +} + +/** + * @param access - represents an access token that can be used to get data from the Khoury Admin OAuth routes + */ +export class AccessToken { + @IsString() + access!: string; +} + +/** + * @param refresh - represents a refresh token that can be used to get new access tokens once the access token expires + */ +export class RefreshToken { + @IsString() + refresh!: string; +} + +/** + * @param code - represents the authorization code that can be used to request access/refresh tokens for a user from the OAuth server + * @param verifier - represents the SHA-256 verifier that was passed in the initial OAuth flow + */ +export class OAuthAccessTokensRequest { + @IsString() + code!: string; + + @IsString() + verifier!: string; +} + export class KhouryDataParams { @IsString() email!: string; @@ -389,6 +440,7 @@ export class KhouryTACourse { instructor!: number; } +// TODO: Get rid of this interface export interface KhouryRedirectResponse { redirect: string; } @@ -807,10 +859,19 @@ export const ERROR_MESSAGES = { }, loginController: { receiveDataFromKhoury: "Invalid request signature", + unableToGetAccessToken: "Unable to retrieve access token", + unabletoRefreshAccessToken: "Unable to refresh access token", + unabletToGetUserInfo: "Unable to get user data", + unableToGetTaInfo: "Unable to get TA data", + unableToGetInstructorCourses: "Unable to get instructor courses", + unableToGetStudentCourses: "Unable to get student courses", + officeHourUserDataError: "Unable to get a user's office hour account", invalidPayload: "The decoded JWT payload is invalid", invalidTempJWTToken: "Error occurred while signing a JWT token", addUserFromKhoury: "Error occurred while translating account from Khoury to Office Hours", + saveUserCourseModel: "Unable to save user course model", + getUserCourseModel: "Unable to get user course model", }, notificationController: { messageNotFromTwilio: "Message not from Twilio", diff --git a/packages/server/.env.development b/packages/server/.env.development index 3d2ee0f2b..d86984a04 100644 --- a/packages/server/.env.development +++ b/packages/server/.env.development @@ -25,4 +25,10 @@ Z4UMR7EOcpfdUE9Hf3m/hs+FUR45uBJeDK1HSFHD8bHKD6kv8FPGfJTotc+2xjJw oYi+1hqp1fIekaxsyQIDAQAB -----END PUBLIC KEY-----' -APPLY_PASSWORD=chungus \ No newline at end of file +APPLY_PASSWORD=chungus +KHOURY_ADMIN_OAUTH_API_URL=https://admin-alpha.khoury.northeastern.edu/api/oauth +KHOURY_ADMIN_OAUTH_URL=https://admin-alpha.khoury.northeastern.edu/oauth +OAUTH_CLIENT_ID=10aac8aac0f1f711756f +OAUTH_CLIENT_SECRET=ZSLHBL5C5CBP87QL +OAUTH_REDIRECT_URI=http://localhost:3000/oauth +OAUTH_SCOPES=scopes=user.info&scopes=ta.info&scopes=student.info&scopes=student.courses&scopes=instructor.courses diff --git a/packages/server/migration/1598733438768-CreateCourseSectionMappingAndRemoveUsername.ts b/packages/server/migration/1598733438768-CreateCourseSectionMappingAndRemoveUsername.ts index 6c6866170..37c56bbcd 100644 --- a/packages/server/migration/1598733438768-CreateCourseSectionMappingAndRemoveUsername.ts +++ b/packages/server/migration/1598733438768-CreateCourseSectionMappingAndRemoveUsername.ts @@ -1,8 +1,7 @@ import { MigrationInterface, QueryRunner } from 'typeorm'; export class CreateCourseSectionMappingAndRemoveUsername1598733438768 - implements MigrationInterface -{ + implements MigrationInterface { name = 'CreateCourseSectionMappingAndRemoveUsername1598733438768'; public async up(queryRunner: QueryRunner): Promise<void> { diff --git a/packages/server/migration/1601671992912-DesktopNotifCreatedAtAndName.ts b/packages/server/migration/1601671992912-DesktopNotifCreatedAtAndName.ts index 05edf2d16..994afa919 100644 --- a/packages/server/migration/1601671992912-DesktopNotifCreatedAtAndName.ts +++ b/packages/server/migration/1601671992912-DesktopNotifCreatedAtAndName.ts @@ -1,8 +1,7 @@ import { MigrationInterface, QueryRunner } from 'typeorm'; export class DesktopNotifCreatedAtAndName1601671992912 - implements MigrationInterface -{ + implements MigrationInterface { name = 'DesktopNotifCreatedAtAndName1601671992912'; public async up(queryRunner: QueryRunner): Promise<void> { diff --git a/packages/server/migration/1609225767925-AddIsProfessorQueueColumn.ts b/packages/server/migration/1609225767925-AddIsProfessorQueueColumn.ts index 9a6eaf9a7..78fc8fd5b 100644 --- a/packages/server/migration/1609225767925-AddIsProfessorQueueColumn.ts +++ b/packages/server/migration/1609225767925-AddIsProfessorQueueColumn.ts @@ -1,8 +1,7 @@ import { MigrationInterface, QueryRunner } from 'typeorm'; export class AddIsProfessorQueueColumn1609225767925 - implements MigrationInterface -{ + implements MigrationInterface { name = 'AddIsProfessorQueueColumn1609225767925'; public async up(queryRunner: QueryRunner): Promise<void> { diff --git a/packages/server/migration/1613852704482-RemoveNameFromUserModel.ts b/packages/server/migration/1613852704482-RemoveNameFromUserModel.ts index 3cd72650c..ce2dfb896 100644 --- a/packages/server/migration/1613852704482-RemoveNameFromUserModel.ts +++ b/packages/server/migration/1613852704482-RemoveNameFromUserModel.ts @@ -1,8 +1,7 @@ import { MigrationInterface, QueryRunner } from 'typeorm'; export class RemoveNameFromUserModel1613852704482 - implements MigrationInterface -{ + implements MigrationInterface { name = 'RemoveNameFromUserModel1613852704482'; public async up(queryRunner: QueryRunner): Promise<void> { diff --git a/packages/server/migration/1619126213146-AddSubmittedCourseFields.ts b/packages/server/migration/1619126213146-AddSubmittedCourseFields.ts index abfcd6d8c..9da3b3cad 100644 --- a/packages/server/migration/1619126213146-AddSubmittedCourseFields.ts +++ b/packages/server/migration/1619126213146-AddSubmittedCourseFields.ts @@ -1,8 +1,7 @@ import { MigrationInterface, QueryRunner } from 'typeorm'; export class AddSubmittedCourseFields1619126213146 - implements MigrationInterface -{ + implements MigrationInterface { name = 'AddSubmittedCourseFields1619126213146'; public async up(queryRunner: QueryRunner): Promise<void> { diff --git a/packages/server/src/healthcheck/healthcheck.controller.ts b/packages/server/src/healthcheck/healthcheck.controller.ts index 822ee22c7..dd8c83fb1 100644 --- a/packages/server/src/healthcheck/healthcheck.controller.ts +++ b/packages/server/src/healthcheck/healthcheck.controller.ts @@ -1,5 +1,6 @@ import { Controller } from '@nestjs/common'; -import { Get } from '@nestjs/common/decorators'; +import { Get, Res } from '@nestjs/common/decorators'; +import { Request, Response } from 'express'; @Controller('healthcheck') export class HealthcheckController { diff --git a/packages/server/src/insights/insights.service.ts b/packages/server/src/insights/insights.service.ts index c395c0c7a..c39530fe1 100644 --- a/packages/server/src/insights/insights.service.ts +++ b/packages/server/src/insights/insights.service.ts @@ -44,8 +44,9 @@ export class InsightsService { convertToInsightsListResponse(insightNames: string[]): ListInsightsResponse { return insightNames.reduce((obj, insightName) => { - const { displayName, description, component, size } = - INSIGHTS_MAP[insightName]; + const { displayName, description, component, size } = INSIGHTS_MAP[ + insightName + ]; return { ...obj, [insightName]: { diff --git a/packages/server/src/login/login-course.service.ts b/packages/server/src/login/login-course.service.ts index 615236f92..1e03b8488 100644 --- a/packages/server/src/login/login-course.service.ts +++ b/packages/server/src/login/login-course.service.ts @@ -5,6 +5,7 @@ import { CourseSectionMappingModel } from 'login/course-section-mapping.entity'; import { UserCourseModel } from 'profile/user-course.entity'; import { UserModel } from 'profile/user.entity'; import { Connection } from 'typeorm'; +import * as Sentry from '@sentry/node'; @Injectable() export class LoginCourseService { @@ -35,7 +36,6 @@ export class LoginCourseService { c.course, c.section, ); - if (course) { const userCourse = await this.courseToUserCourse( user.id, @@ -49,13 +49,18 @@ export class LoginCourseService { if (info.ta_courses) { for (const c of info.ta_courses) { // Query for all the courses which match the name of the generic course from Khoury - const courseMappings = ( - await CourseSectionMappingModel.find({ - where: { genericCourseName: c.course }, // TODO: Add semester support - relations: ['course'], - }) - ).filter((cm) => cm.course.enabled); - + let courseMappings: CourseSectionMappingModel[]; + try { + courseMappings = ( + await CourseSectionMappingModel.find({ + where: { genericCourseName: c.course }, // TODO: Add semester support + relations: ['course'], + }) + ).filter((cm) => cm.course.enabled); + } catch (err) { + Sentry.captureException(err); + console.error(err); + } for (const courseMapping of courseMappings) { const taCourse = await this.courseToUserCourse( user.id, @@ -66,7 +71,6 @@ export class LoginCourseService { } } } - // Delete "stale" user courses for (const previousCourse of user.courses) { if ( @@ -80,7 +84,6 @@ export class LoginCourseService { } } } - user.courses = userCourses; await user.save(); return user; @@ -90,13 +93,18 @@ export class LoginCourseService { courseName: string, courseSection: number, ): Promise<CourseModel> { - const courseSectionModel = ( - await CourseSectionMappingModel.find({ - where: { genericCourseName: courseName, section: courseSection }, - relations: ['course'], - }) - ).find((cm) => cm.course.enabled); - + let courseSectionModel: CourseSectionMappingModel; + try { + courseSectionModel = ( + await CourseSectionMappingModel.find({ + where: { genericCourseName: courseName, section: courseSection }, + relations: ['course'], + }) + ).find((cm) => cm.course.enabled); + } catch (err) { + Sentry.captureException(err); + console.log(err); + } return courseSectionModel?.course; } @@ -111,14 +119,24 @@ export class LoginCourseService { }); if (userCourse && userCourse.override && userCourse.role === role) { userCourse.override = false; - await userCourse.save(); + try { + await userCourse.save(); + } catch (err) { + Sentry.captureException(err); + console.error(err); + } } if (!userCourse) { - userCourse = await UserCourseModel.create({ - userId, - courseId, - role, - }).save(); + try { + userCourse = await UserCourseModel.create({ + userId, + courseId, + role, + }).save(); + } catch (err) { + Sentry.captureException(err); + console.error(err); + } } return userCourse; } diff --git a/packages/server/src/login/login.controller.ts b/packages/server/src/login/login.controller.ts index 1983fc96f..9f8de736c 100644 --- a/packages/server/src/login/login.controller.ts +++ b/packages/server/src/login/login.controller.ts @@ -1,9 +1,18 @@ import { + KhouryDataParams, + OAuthAccessTokensRequest, + OAuthAccessTokensResponse, + RefreshToken, + AccessToken, ERROR_MESSAGES, + KhouryTACourse, GetSelfEnrollResponse, - KhouryDataParams, - KhouryRedirectResponse, Role, + KHOURY_ADMIN_OAUTH_API_URL, + OAUTH_CLIENT_ID, + OAUTH_REDIRECT_URI, + OAUTH_SCOPES, + OAUTH_CLIENT_SECRET, } from '@koh/common'; import { BadRequestException, @@ -33,6 +42,7 @@ import { UserModel } from 'profile/user.entity'; import { Connection } from 'typeorm'; import { NonProductionGuard } from '../guards/non-production.guard'; import { LoginCourseService } from './login-course.service'; +import axios from 'axios'; @Controller() export class LoginController { @@ -43,82 +53,327 @@ export class LoginController { private configService: ConfigService, ) {} - @Post('/khoury_login') - async recieveDataFromKhoury( + /** + * Gets Access and Refresh tokens for the current user attempting to login using the passed temporary authorization code. + * + * @param res The response obkect + * @param body The request body that contains the access code that will be used to get access and refresh tokens + * @returns A pair value of Access and Refresh tokens + */ + @Post('/oauth/tokens') + async getAccessTokens( + @Res() res: Response, + @Body() body: OAuthAccessTokensRequest, + ): Promise<OAuthAccessTokensResponse> { + const authCode = body.code; + const challenge = body.verifier; + const isSecure = this.configService + .get<string>('DOMAIN') + .startsWith('https://'); + const token = axios + .post( + `${KHOURY_ADMIN_OAUTH_API_URL}/token?client_id=${OAUTH_CLIENT_ID}&client_secret=${OAUTH_CLIENT_SECRET}&grant_type=authorization_code&redirect_uri=${OAUTH_REDIRECT_URI}&code=${authCode}&${OAUTH_SCOPES}&verifier=${challenge}`, + ) + .then((token) => { + const tokens = { + access: token.data.access, + refresh: token.data.refresh, + }; + res.cookie('oauth_access', tokens.access, { + httpOnly: true, + secure: isSecure, + }); + res.cookie('oauth_refresh', tokens.refresh, { + httpOnly: true, + secure: isSecure, + }); + res.json(tokens); + return tokens; + }) + .catch((e) => { + console.error(e); + Sentry.captureException( + 'Error while getting Access and Refresh tokens: ' + e, + ); + throw new HttpException( + ERROR_MESSAGES.loginController.unableToGetAccessToken, + HttpStatus.BAD_REQUEST, + ); + }); + return token; + } + + /** + * Gets a new access token using the paired refresh token. + * + * @param res The response object + * @param body The refresh token being used to request a new access token + * @returns A new access token + */ + @Post('/oauth/tokens/refresh') + async refreshAccessTokens( + @Res() res: Response, + @Body() body: RefreshToken, + ): Promise<AccessToken> { + const refreshToken = body.refresh; + const isSecure = this.configService + .get<string>('DOMAIN') + .startsWith('https://'); + `${KHOURY_ADMIN_OAUTH_API_URL}/token/refresh? + client_id=${OAUTH_CLIENT_ID} + &refresh_token=${refreshToken} + &client_secret=${OAUTH_CLIENT_SECRET} + &grant_type=refresh_token + &redirect_uri=${OAUTH_REDIRECT_URI} + &${OAUTH_SCOPES}`; + + const token = axios + .get( + `${KHOURY_ADMIN_OAUTH_API_URL}/token/refresh?client_id=${OAUTH_CLIENT_ID}&refresh_token=${refreshToken}&client_secret=${OAUTH_CLIENT_SECRET}&grant_type=refresh_token&redirect_uri=${OAUTH_REDIRECT_URI}&${OAUTH_SCOPES}`, + ) + .then((token) => { + const tokens = { + access: token.data.access, + }; + res.cookie('oauth_access', tokens.access, { + httpOnly: true, + secure: isSecure, + }); + res.json(tokens); + return tokens; + }) + .catch((e) => { + console.error(e); + Sentry.captureException('Error while getting Refresh token: ' + e); + throw new HttpException( + ERROR_MESSAGES.loginController.unabletoRefreshAccessToken, + HttpStatus.BAD_REQUEST, + ); + }); + return token; + } + + /** + * Gets a user from the Khoury server and maps the user's account/courses into the Office Hours database. + * + * @param res The request object + * @param body The Access token used to get a user's information from the Khoury server + * @param req The request object + */ + @Post('/oauth/user') + async getUser( + @Res() res: Response, + @Body() body: AccessToken, @Req() req: Request, - @Body() body: KhouryDataParams, - ): Promise<KhouryRedirectResponse> { - if (process.env.NODE_ENV === 'production') { - // Check that request has come from Khoury - const parsedRequest = httpSignature.parseRequest(req); - const verifySignature = httpSignature.verifyHMAC( - parsedRequest, - this.configService.get('KHOURY_PRIVATE_KEY'), - ); - if (!verifySignature) { - Sentry.captureMessage('Invalid request signature: ' + parsedRequest); - throw new UnauthorizedException('Invalid request signature'); - } + ): Promise<void> { + let authorizationToken = 'Bearer ' + body.access; + let request; + try { + request = await axios.get( + KHOURY_ADMIN_OAUTH_API_URL + `/userinfo/read/`, + { + headers: { + Authorization: authorizationToken, + }, + }, + ); + } catch (err) { + console.error( + 'Error while retrieving user data from Khoury server: ' + err, + ); + Sentry.captureException( + 'Error while retrieving user data from Khoury server: ' + err, + ); + throw new HttpException( + ERROR_MESSAGES.loginController.unabletToGetUserInfo, + HttpStatus.BAD_REQUEST, + ); } - let user; + let khouryData = { + first_name: request.data.firstname, + last_name: request.data.lastname, + email: request.data.email, + accountType: request.data.account_type, + campus: request.data.campus, + ta_courses: [], + courses: [], + photo_url: '', + }; + + // This is a student signing in so get the students list of courses + if (khouryData.accountType.includes('student')) { + khouryData.courses = await this.getCourses(authorizationToken); + + // Get the courses the singing in student TA's for + khouryData.ta_courses = await this.getTACourses( + authorizationToken, + `/tainfo/read/`, + true, + ); + + // An instructor is signing in, get an instructor's courses and map it as the khoury Data TA courses + } else if (khouryData.accountType.includes('faculty')) { + khouryData.ta_courses = await this.getTACourses( + authorizationToken, + '/instructorcourses/read/', + false, + ); + } + + this.signInToOfficeHoursUser(khouryData) + .then((id) => { + this.createUserToken(id, res); + }) + .catch((err) => { + console.error('Error while signing user in: ' + err); + Sentry.captureException('Error while signing user in: ' + err); + throw new HttpException( + ERROR_MESSAGES.loginController.officeHourUserDataError, + HttpStatus.BAD_REQUEST, + ); + }); + } + + /** + * Gets the current student list of courses they are enrolled in + * + * @param accessToken The token used to get the student's current courses + * @returns The list of a student courses + */ + private async getCourses(accessToken: string) { + let request; + let courses = []; try { - user = await this.loginCourseService.addUserFromKhoury(body); - } catch (e) { - Sentry.captureException(e); - console.error('Khoury login threw an exception, the body was ', body); - console.error(e); + request = await axios.get( + KHOURY_ADMIN_OAUTH_API_URL + '/studentcourses/read/', + { + headers: { + Authorization: accessToken, + }, + }, + ); + } catch (err) { + console.error("Error while getting a student's courses: " + err); + Sentry.captureException( + "Error while getting a student's courses: " + err, + ); throw new HttpException( - ERROR_MESSAGES.loginController.addUserFromKhoury, - HttpStatus.INTERNAL_SERVER_ERROR, + ERROR_MESSAGES.loginController.unableToGetStudentCourses, + HttpStatus.BAD_REQUEST, ); } - // Create temporary login token to send user to. - const token = await this.jwtService.signAsync( - { userId: user.id }, - { expiresIn: 60 }, - ); + for (const course of request.data.courses) { + courses.push({ + course: course.course, + semester: course.semester, + campus: course.campus, + section: course.section, + crn: course.crn, + title: course.title, + accelerated: course.accelerated, + }); + } + return courses; + } - if (token === null || token === undefined) { - console.error('Temporary JWT is invalid'); + /** + * Returns the TA courses a student is in or an instructors courses + * + * @param accessToken The token used to access the courses resource + * @param url The URL to make a request to + * @param isStudent Whether the user is a student or an instructor + * @returns A list of TA courses for the student or instructor + */ + private async getTACourses( + accessToken: string, + url: string, + isStudent: boolean, + ): Promise<KhouryTACourse[]> { + let request; + let courses = []; + try { + request = await axios.get(KHOURY_ADMIN_OAUTH_API_URL + url, { + headers: { + Authorization: accessToken, + }, + }); + } catch (err) { + console.error("Error while getting a TA's courses: " + err); + Sentry.captureException("Error while getting a TA's courses: " + err); throw new HttpException( - ERROR_MESSAGES.loginController.invalidTempJWTToken, - HttpStatus.INTERNAL_SERVER_ERROR, + ERROR_MESSAGES.loginController.unableToGetTaInfo, + HttpStatus.BAD_REQUEST, ); } - - return { - redirect: - this.configService.get('DOMAIN') + `/api/v1/login/entry?token=${token}`, - }; + if (isStudent) { + /* + The 'request.data.is_ta' has to be a separate check because if the account is a student but they are not a TA we do nothing. + If we did (isStudent && request.data.is_ta == true) then the above scenario would break as the code will go into the else clause + which is meant for ONLY instructors because they do not have the .is_ta check. + */ + if (request.data.is_ta === true) { + for (const course of request.data.courses) { + courses.push({ + course: course.course, + semester: course.semester, + campus: course.campus, + }); + } + } + } else { + for (const course of request.data.courses) { + courses.push({ + course: course.course, + semester: course.semester, + campus: course.campus, + }); + } + } + return courses; } - // NOTE: Although the two routes below are on the backend, - // they are meant to be visited by the browser so a cookie can be set - - // This is the real admin entry point - @Get('/login/entry') - async enterFromKhoury( - @Res() res: Response, - @Query('token') token: string, - ): Promise<void> { + /** + * Creates an Office Hour JWT based on the user ID. + * + * @param userId The ID of the logging in user + * @param res The response object + */ + private async createUserToken(userId: string, res: Response) { + // Create temporary login token to send user to. + const token = await this.jwtService.signAsync( + { userId: userId }, + { expiresIn: 60 }, + ); const isVerified = await this.jwtService.verifyAsync(token); - if (!isVerified) { throw new UnauthorizedException(); } - const payload = this.jwtService.decode(token) as { userId: number }; + this.enter(res, payload.userId); + } - if (payload === null || payload === undefined) { - console.error('Decoded JWT is invalid'); - throw new HttpException( - ERROR_MESSAGES.loginController.invalidPayload, - HttpStatus.INTERNAL_SERVER_ERROR, + /** + * Adds the user from the Khoury server to the Office Hours database + * + * @param data The data received from the Khoury server regarding the user + * @returns The ID of the user + */ + private async signInToOfficeHoursUser( + data: KhouryDataParams, + ): Promise<string> { + let user; + try { + user = await this.loginCourseService.addUserFromKhoury(data); + } catch (e) { + console.error('Khoury login threw an exception, the body was ', data); + console.error(e); + Sentry.captureException( + 'Error while performing all the course mappings for the user: ' + e, ); + throw e; } - - this.enter(res, payload.userId); + return user.id; } // This is for login on development only @@ -167,8 +422,16 @@ export class LoginController { @Get('self_enroll_courses') async selfEnrollEnabledAnywhere(): Promise<GetSelfEnrollResponse> { - const courses = await CourseModel.find(); - return { courses: courses.filter((course) => course.selfEnroll) }; + try { + const courses = await CourseModel.find(); + return { courses: courses.filter((course) => course.selfEnroll) }; + } catch (err) { + console.error(err); + throw new HttpException( + ERROR_MESSAGES.loginController.getUserCourseModel, + HttpStatus.INTERNAL_SERVER_ERROR, + ); + } } @Post('create_self_enroll_override/:id') @@ -198,12 +461,20 @@ export class LoginController { ); } - await UserCourseModel.create({ - userId: user.id, - courseId: courseId, - role: Role.STUDENT, - override: true, - expires: true, - }).save(); + try { + await UserCourseModel.create({ + userId: user.id, + courseId: courseId, + role: Role.STUDENT, + override: true, + expires: true, + }).save(); + } catch (err) { + console.error(err); + throw new HttpException( + ERROR_MESSAGES.loginController.saveUserCourseModel, + HttpStatus.INTERNAL_SERVER_ERROR, + ); + } } } diff --git a/packages/server/src/notification/desktop-notif-subscriber.ts b/packages/server/src/notification/desktop-notif-subscriber.ts index 5281a36de..5aee2f41c 100644 --- a/packages/server/src/notification/desktop-notif-subscriber.ts +++ b/packages/server/src/notification/desktop-notif-subscriber.ts @@ -9,8 +9,7 @@ import { NotificationService } from './notification.service'; @EventSubscriber() export class DesktopNotifSubscriber - implements EntitySubscriberInterface<DesktopNotifModel> -{ + implements EntitySubscriberInterface<DesktopNotifModel> { notifService: NotificationService; constructor(connection: Connection, notifService: NotificationService) { this.notifService = notifService; diff --git a/packages/server/src/notification/notification.service.ts b/packages/server/src/notification/notification.service.ts index 1c88b8924..f9110fe36 100644 --- a/packages/server/src/notification/notification.service.ts +++ b/packages/server/src/notification/notification.service.ts @@ -18,7 +18,8 @@ export const NotifMsgs = { "You've unregistered from text notifications for Khoury Office Hours. Feel free to re-register any time through the website", DUPLICATE: "You've already been verified to receive text notifications from Khoury Office Hours!", - OK: 'Thank you for verifying your number with Khoury Office Hours! You are now signed up for text notifications!', + OK: + 'Thank you for verifying your number with Khoury Office Hours! You are now signed up for text notifications!', }, queue: { ALERT_BUTTON: diff --git a/packages/server/src/question/question.subscriber.ts b/packages/server/src/question/question.subscriber.ts index 720e402e6..1d1df89cb 100644 --- a/packages/server/src/question/question.subscriber.ts +++ b/packages/server/src/question/question.subscriber.ts @@ -17,8 +17,7 @@ import { QuestionModel } from './question.entity'; @EventSubscriber() export class QuestionSubscriber - implements EntitySubscriberInterface<QuestionModel> -{ + implements EntitySubscriberInterface<QuestionModel> { private notifService: NotificationService; private queueSSEService: QueueSSEService; constructor( diff --git a/packages/server/src/queue/queue-clean/queue-clean.service.ts b/packages/server/src/queue/queue-clean/queue-clean.service.ts index 075951470..6330cb49f 100644 --- a/packages/server/src/queue/queue-clean/queue-clean.service.ts +++ b/packages/server/src/queue/queue-clean/queue-clean.service.ts @@ -23,17 +23,16 @@ export class QueueCleanService { @Cron(CronExpression.EVERY_DAY_AT_MIDNIGHT) async cleanAllQueues(): Promise<void> { - const queuesWithOpenQuestions: QueueModel[] = - await QueueModel.getRepository() - .createQueryBuilder('queue_model') - .leftJoinAndSelect('queue_model.questions', 'question') - .where('question.status IN (:...status)', { - status: [ - ...Object.values(OpenQuestionStatus), - ...Object.values(LimboQuestionStatus), - ], - }) - .getMany(); + const queuesWithOpenQuestions: QueueModel[] = await QueueModel.getRepository() + .createQueryBuilder('queue_model') + .leftJoinAndSelect('queue_model.questions', 'question') + .where('question.status IN (:...status)', { + status: [ + ...Object.values(OpenQuestionStatus), + ...Object.values(LimboQuestionStatus), + ], + }) + .getMany(); // Clean 1 queue at a time await async.mapLimit( @@ -45,8 +44,9 @@ export class QueueCleanService { @Cron(CronExpression.EVERY_DAY_AT_3AM) public async checkoutAllStaff(): Promise<void> { - const queuesWithCheckedInStaff: QueueModel[] = - await QueueModel.getRepository().find({ relations: ['staffList'] }); + const queuesWithCheckedInStaff: QueueModel[] = await QueueModel.getRepository().find( + { relations: ['staffList'] }, + ); queuesWithCheckedInStaff.forEach(async (queue) => { if (!(await queue.areThereOfficeHoursRightNow())) { diff --git a/packages/server/test/course.integration.ts b/packages/server/test/course.integration.ts index acf13e125..00e126f7e 100644 --- a/packages/server/test/course.integration.ts +++ b/packages/server/test/course.integration.ts @@ -440,7 +440,7 @@ describe('Course Integration', () => { }) .expect(200); - const checkinTimes = (data.body as unknown as TACheckinTimesResponse) + const checkinTimes = ((data.body as unknown) as TACheckinTimesResponse) .taCheckinTimes; const taName = ta.firstName + ' ' + ta.lastName; diff --git a/packages/server/test/login.integration.ts b/packages/server/test/login.integration.ts index 786ca78fc..37652c277 100644 --- a/packages/server/test/login.integration.ts +++ b/packages/server/test/login.integration.ts @@ -1,7 +1,10 @@ import { Role } from '@koh/common'; import { JwtService } from '@nestjs/jwt'; import { TestingModuleBuilder } from '@nestjs/testing'; +import { LoginCourseService } from 'login/login-course.service'; +import { LoginController } from 'login/login.controller'; import { UserCourseModel } from 'profile/user-course.entity'; +import { Connection, createConnection } from 'typeorm'; import { LoginModule } from '../src/login/login.module'; import { UserModel } from '../src/profile/user.entity'; import { @@ -25,6 +28,15 @@ describe('Login Integration', () => { t.overrideProvider(JwtService).useValue(mockJWT), ); + describe('GET /logout', () => { + it('makes sure logout endpoint is destroying cookies like a mob boss', async () => { + const res = await supertest().get(`/logout`).expect(302); + expect(res.header['location']).toBe('/login'); + expect(res.get('Set-Cookie')[0]).toContain('auth_token=;'); + }); + }); + + /* describe('POST /login/entry', () => { it('request to entry with correct jwt payload works', async () => { const token = await mockJWT.signAsync({ userId: 1 }); @@ -57,34 +69,7 @@ describe('Login Integration', () => { }); }); - describe('POST /khoury_login', () => { - it('creates user and sends back correct redirect', async () => { - const user = await UserModel.findOne({ - where: { email: 'stenzel.w@northeastern.edu' }, - }); - expect(user).toBeUndefined(); - - const res = await supertest().post('/khoury_login').send({ - email: 'stenzel.w@northeastern.edu', - campus: 1, - first_name: 'Will', - last_name: 'Stenzel', - photo_url: 'sdf', - courses: [], - ta_courses: [], - }); - - // Expect that the new user has been created - const newUser = await UserModel.findOne({ - where: { email: 'stenzel.w@northeastern.edu' }, - }); - expect(newUser).toBeDefined(); - // And that the redirect is correct - expect(res.body).toEqual({ - redirect: 'http://localhost:3000/api/v1/login/entry?token={"userId":1}', - }); - }); it('converts husky emails to northeastern emails', async () => { const user = await UserModel.findOne({ @@ -526,12 +511,5 @@ describe('Login Integration', () => { expect(ucms.every((ucm) => ucm.role === Role.PROFESSOR)).toBeTruthy(); }); }); - - describe('GET /logout', () => { - it('makes sure logout endpoint is destroying cookies like a mob boss', async () => { - const res = await supertest().get(`/logout`).expect(302); - expect(res.header['location']).toBe('/login'); - expect(res.get('Set-Cookie')[0]).toContain('auth_token=;'); - }); - }); +*/ }); diff --git a/yarn.lock b/yarn.lock index 253b88859..fc48a88e4 100644 --- a/yarn.lock +++ b/yarn.lock @@ -10721,6 +10721,11 @@ node-fetch@^2.6.0: resolved "https://registry.yarnpkg.com/node-fetch/-/node-fetch-2.6.1.tgz#045bd323631f76ed2e2b55573394416b639a0052" integrity sha512-V4aYg89jEoVRxRb2fJdAg8FHvI7cEyYdVAh94HH0UIK8oJxUfkjlDQN9RbMx+bEjP7+ggMiFRprSti032Oipxw== +node-forge@^0.10.0: + version "0.10.0" + resolved "https://registry.yarnpkg.com/node-forge/-/node-forge-0.10.0.tgz#32dea2afb3e9926f02ee5ce8794902691a676bf3" + integrity sha512-PPmu8eEeG9saEUvI97fm4OYxXVB6bFvyNTyiUOBichBpFG8A1Ljw3bY62+5oOjDEMHRnd0Y7HQ+x7uzxOzC6JA== + node-ical@^0.11.3: version "0.11.3" resolved "https://registry.yarnpkg.com/node-ical/-/node-ical-0.11.3.tgz#fb7891547eb711545513b92938b1efae0aba832b"