BE OAuth Configuration #858
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| Learn about Next.js in an interactive course with quizzes! | ||
| </p> | ||
| </a> | ||
| import { useEffect } from "react"; |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
| </p> | ||
| </a> | ||
| import { useEffect } from "react"; | ||
| import { calendarAuthRedirect } from "@generatesac/lib"; |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
| const rootReducer = combineReducers({ | ||
| user: userReducer, | ||
| [baseApi.reducerPath]: baseApi.reducer, | ||
| clubReducer: clubReducer, |
Check warning
Code scanning / CodeQL
Variable not declared before use Warning
| user: userReducer, | ||
| [baseApi.reducerPath]: baseApi.reducer, | ||
| clubReducer: clubReducer, | ||
| recruitmentReducer: recruitmentReducer, |
Check warning
Code scanning / CodeQL
Variable not declared before use Warning
|
A lot of stuff needs to be removed / moved around |
| } | ||
|
|
||
| // Return the authorization URL: | ||
| func (client OAuthClient) Authorize(db *gorm.DB, jwt auth.JWTClientInterface, userId *uuid.UUID) (*string, error) { |
There was a problem hiding this comment.
we should probably be using pointer receivers over value receivers so we don't have to copy the config strings around
| func (client OAuthClient) Authorize(db *gorm.DB, jwt auth.JWTClientInterface, userId *uuid.UUID) (*string, error) { | |
| func (client *OAuthClient) Authorize(db *gorm.DB, jwt auth.JWTClientInterface, userId *uuid.UUID) (*string, error) { |
| **/ | ||
| func (client GoogleOAuthClient) AuthorizeURL(state string) string { | ||
| return ( | ||
| client.OAuthConfig.BaseURL + "/auth?" + |
There was a problem hiding this comment.
there might be a url builder utility we can use here. if not, we should probably prefer fmt.Sprintf over string concat
|
|
||
| func CreateCalendarToken(db *gorm.DB, jwtClient auth.JWTClientInterface, userID *uuid.UUID) (*string, error) { | ||
| // Generate CSRF token: | ||
| csrfToken, err := jwtClient.GenerateToken(auth.Claims{ |
There was a problem hiding this comment.
to separate concerns on token generation and database transactions, the token should probably be generated in the service, then passed to this function as a string
| @@ -246,4 +247,11 @@ CREATE TABLE IF NOT EXISTS verifications( | |||
|
|
|||
| CREATE UNIQUE INDEX IF NOT EXISTS uni_verifications_token ON verifications USING btree ("token"); | |||
|
|
|||
| CREATE TABLE IF NOT EXISTS user_calendar_token( | |||
There was a problem hiding this comment.
let's update the [id].down.sql to include a matching drop statement
There was a problem hiding this comment.
nit but we should split into files for google and outlook. including the associated request bodies there as well
There was a problem hiding this comment.
is there not an associated service and transactions? we should separate our concerns so integrations/oauth only handles the bespoke google/outlook requirements while we have our internal service and transaction logic in this package
didn't see this until after my review, sorry for redundant comments on this |
Description
[Link to Ticket](insert the link to your ticket inside the parenthesis here)
Please include a summary of the changes and the related issue. Please also
include relevant motivation, context, and images!
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. If they are unit
tests, provide the file name the tests are in. If they are not unit tests,
describe how you tested the change.
Checklist