-
Notifications
You must be signed in to change notification settings - Fork 13
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
Office Hours Khoury OAuth Implementation #683
Draft
jtavera235
wants to merge
15
commits into
sandboxnu:master
Choose a base branch
from
jtavera235:oauth-impl
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
fe637d5
Implemented new endpoint and successfully logs in
jtavera235 4413f6d
Broke down OAuth user into function and fixed error handling
jtavera235 8762443
Claened up OAuth feature
jtavera235 7512fb9
Started unit testing
jtavera235 1e0a560
Added courses implementation
jtavera235 90fb128
Refactored code
jtavera235 da0c068
Added error handling for login, works with alpha server
jtavera235 087b98e
Fixed course mappings and started adding error handling to endpoints
jtavera235 82b21f7
Cleaned up code a little bit
jtavera235 70ba668
Fixed merge conflicts
jtavera235 2b839f7
Added error handling
jtavera235 3435f4e
Added code comments
jtavera235 f86b5f1
Updated page
jtavera235 9669887
Finished Outh impl and re-did env var
jtavera235 ea478e5
Removed dev variables
jtavera235 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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> | ||
); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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, | ||
}; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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://[email protected]/5557379", | ||
dsn: | ||
"https://[email protected]/5557379", | ||
tracesSampleRate: 0.2, | ||
}); | ||
} | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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={() => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i dont think you have to wrap this function in a lambda, you can just pass it in as data |
||
openKhouryOAuthLoginPage(); | ||
}} | ||
> | ||
Log in via Khoury Admin | ||
</Button> | ||
</ContentContainer> | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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> | ||
); | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to change it whenever we re-visit this, but can this be a string literal?