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.

Sign up for GitHub

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

Jump to bottom

Seacat Auth API refactoring #42

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Seacat Auth API refactoring #42

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c608551
prefix account endpoints with /account
byewokko Nov 16, 2023
b813bb8
fix wrong path
byewokko Nov 20, 2023
447a7c3
change login.prologue to login-prologue
byewokko Nov 20, 2023
24647ec
change last_login to last-login
byewokko Nov 20, 2023
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
2 changes: 1 addition & 1 deletion src/modules/auth/containers/LoginCard.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ function LoginCard(props) {
let SeaCatAuthPrologueAPI = props.app.axiosCreate('seacat-auth');
let response;
try {
response = await SeaCatAuthPrologueAPI.put('/public/login.prologue', jwk)
response = await SeaCatAuthPrologueAPI.put('/public/login-prologue', jwk)
} catch (e) {
props.app.addAlert(
"danger",
Expand Down
4 changes: 2 additions & 2 deletions src/modules/auth/email/EmailScreen.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ function ManageEmailCard(props) {
let redirect_uri = params.get("redirect_uri");

let history = useHistory();
let SeaCatAuthAPI = props.app.axiosCreate('seacat-auth');
let SeaCatAccountAPI = props.app.axiosCreate('seacat-auth/account');
Copy link
Collaborator

Choose a reason for hiding this comment

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

@byewokko I dont think this is a good way to do this. Here should be just a service name. So only seacat-auth here should be correct.

This is the correct way:

response = await SeaCatAccountAPI.put("/account/credentials", ...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the idea was to have a separate axios instance for login and auth stuff (seacat-auth/public) and another one for the account management (seacat-auth/account). they now even have different authorization and ports on backend. i wanted to make this separation clear in the code:

let SeaCatAccountAPI = props.app.axiosCreate('seacat-auth/account');
let SeaCatPublicAPI = props.app.axiosCreate('seacat-auth/public');

but maybe it is not necessary. what does @ateska think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As discussed: Franta is right. There is no need for separation, also the proposed changes would make the code harder to search and correlate with the nginx config. So the correct approach is this:

let SeaCatAuthAPI = props.app.axiosCreate('seacat-auth');
...
response = await SeaCatAuthAPI.put("/account/credentials", ...)

let email = props.userinfo?.email;
let number = props.userinfo?.phone ? props.userinfo.phone : "";

Expand All @@ -66,7 +66,7 @@ function ManageEmailCard(props) {
const onSubmit = async (values) => {
let response;
try {
response = await SeaCatAuthAPI.put("/public/credentials",
response = await SeaCatAccountAPI.put("/credentials",
JSON.stringify(values),
{ headers: {
'Content-Type': 'application/json'
Expand Down
9 changes: 5 additions & 4 deletions src/modules/auth/home/HomeScreen.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ function HomeScreen(props) {
const { t } = useTranslation();
const history = useHistory();
const SeaCatAuthAPI = props.app.axiosCreate('seacat-auth');
const SeaCatAccountAPI = props.app.axiosCreate('seacat-auth/account');

generatePenrose();

Expand Down Expand Up @@ -70,7 +71,7 @@ function HomeScreen(props) {

const fetchUpdateFeatures = async () => {
try {
const response = await SeaCatAuthAPI.get("/public/provider");
const response = await SeaCatAccountAPI.get("/provider");

if (response.data.result != "OK") throw response;

Expand All @@ -87,7 +88,7 @@ function HomeScreen(props) {

const fetchLastLogin = async () => {
try {
const response = await SeaCatAuthAPI.get("/public/last_login");
const response = await SeaCatAccountAPI.get("/last-login");
setLastLogin(response.data);
} catch (e) {
console.error(e);
Expand Down Expand Up @@ -148,7 +149,7 @@ function HomeScreen(props) {
const logoutAll = async () => {
let response;
try {
response = await SeaCatAuthAPI.delete('/public/sessions');
response = await SeaCatAccountAPI.delete('/sessions');
if (response.data.result !== "OK") {
throw new Error(t("HomeScreen|Something went wrong when logging you out from all devices"));
}
Expand Down Expand Up @@ -188,7 +189,7 @@ function HomeScreen(props) {
// Remove external service method
const removeExternalService = async (provider) => {
try {
await SeaCatAuthAPI.delete("/public/ext-login/" + provider);
await SeaCatAccountAPI.delete("/ext-login/" + provider);
props.app.addAlert("success", t("HomeScreen|Service was successfully disconnected"));
// reload in order to get updated userinfo
window.location.reload();
Expand Down
4 changes: 2 additions & 2 deletions src/modules/auth/number/PhoneNumberScreen.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ function ManageNumberCard(props) {
let redirect_uri = params.get("redirect_uri");

let history = useHistory();
let SeaCatAuthAPI = props.app.axiosCreate('seacat-auth');
let SeaCatAccountAPI = props.app.axiosCreate('seacat-auth/account');
let number = props.userinfo?.phone;
let email = props.userinfo?.email ? props.userinfo.email : "";

Expand All @@ -68,7 +68,7 @@ function ManageNumberCard(props) {
const onSubmit = async (values) => {
let response;
try {
response = await SeaCatAuthAPI.put("/public/credentials",
response = await SeaCatAccountAPI.put("/credentials",
JSON.stringify(values),
{ headers: {
'Content-Type': 'application/json'
Expand Down
8 changes: 4 additions & 4 deletions src/modules/auth/otp/TOTPScreen.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ function SetTOTPCard(props) {
const { t, i18n } = useTranslation();
const { handleSubmit, register, getValues, formState: { errors } } = useForm();
let history = useHistory();
let SeaCatAuthAPI = props.app.axiosCreate('seacat-auth');
let SeaCatAccountAPI = props.app.axiosCreate('seacat-auth/account');

let params = new URLSearchParams(useLocation().search);
let redirect_uri = params.get("redirect_uri");
Expand Down Expand Up @@ -71,7 +71,7 @@ function SetTOTPCard(props) {
const getData = async () => {
let response;
try {
response = await SeaCatAuthAPI.get("/public/totp");
response = await SeaCatAccountAPI.get("/totp");
if (response.data.result == 'OK') {
setActive(response.data.active);
setConfigURL(response.data.url);
Expand All @@ -91,7 +91,7 @@ function SetTOTPCard(props) {
let response;
if (active) {
try {
response = await SeaCatAuthAPI.put("/public/unset-totp");
response = await SeaCatAccountAPI.put("/unset-totp");
if (response.data.result !== "OK") {
throw new Error(t("TOTPScreen|Something went wrong, can't deactivate OTP"));
}
Expand All @@ -117,7 +117,7 @@ function SetTOTPCard(props) {

} else {
try {
response = await SeaCatAuthAPI.put("/public/set-totp",
response = await SeaCatAccountAPI.put("/set-totp",
JSON.stringify(values),
{ headers: {
'Content-Type': 'application/json'
Expand Down
4 changes: 2 additions & 2 deletions src/modules/auth/passwd/ChangePwdScreen.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,11 @@ function ChangePwdCard(props) {
});

const onSubmit = async (values) => {
let SeaCatAuthAPI = props.app.axiosCreate('seacat-auth');
let SeaCatAccountAPI = props.app.axiosCreate('seacat-auth/account');
let response;

try {
response = await SeaCatAuthAPI.put("/public/password-change", values)
response = await SeaCatAccountAPI.put("/password-change", values)
} catch (e) {
props.app.addAlert("danger", `${t("ChangePwdScreen|Something went wrong")}. ${e?.response?.data?.message}`, 30);
return;
Expand Down
12 changes: 6 additions & 6 deletions src/modules/auth/webauthn/WebAuthnScreen.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export default function WebAuthnScreen(props) {
function WebAuthnCard(props) {
const { t, i18n } = useTranslation();
let history = useHistory();
let SeaCatAuthAPI = props.app.axiosCreate('seacat-auth');
let SeaCatAccountAPI = props.app.axiosCreate('seacat-auth/account');

let params = new URLSearchParams(useLocation().search);
let redirect_uri = params.get("redirect_uri");
Expand Down Expand Up @@ -69,7 +69,7 @@ function WebAuthnCard(props) {
const getAuthenticators = async () => {
let response;
try {
response = await SeaCatAuthAPI.get('/public/webauthn');
response = await SeaCatAccountAPI.get('/webauthn');
// TODO: enable validation, when ready in SA service
if (response.data.result != 'OK') {
throw new Error(t("WebAuthnScreen|Something went wrong, can't retrieve authenticators"));
Expand Down Expand Up @@ -97,7 +97,7 @@ function WebAuthnCard(props) {
// Get register options for WebAuthn
let response;
try {
response = await SeaCatAuthAPI.get('/public/webauthn/register-options');
response = await SeaCatAccountAPI.get('/webauthn/register-options');
// TODO: enable validation, when ready in SA service
// if (response.data.result != 'OK') {
// throw new Error(t("WebAuthnScreen|Something went wrong, registration of authenticator failed"));
Expand Down Expand Up @@ -151,7 +151,7 @@ function WebAuthnCard(props) {
// Register credentials
let registerResponse;
try {
registerResponse = await SeaCatAuthAPI.put('/public/webauthn/register', credToJSON);
registerResponse = await SeaCatAccountAPI.put('/webauthn/register', credToJSON);
if (registerResponse.data.result != 'OK') {
throw new Error(t("WebAuthnScreen|Something went wrong, registration of authenticator failed"));
}
Expand All @@ -178,7 +178,7 @@ function WebAuthnCard(props) {
const onUnregister = async (id) => {
let response;
try {
response = await SeaCatAuthAPI.delete(`/public/webauthn/${id}`);
response = await SeaCatAccountAPI.delete(`/webauthn/${id}`);
// TODO: enable validation, when ready in SA service
if (response.data.result != 'OK') {
throw new Error(t("WebAuthnScreen|Something went wrong, can't unregister authenticator"));
Expand All @@ -203,7 +203,7 @@ function WebAuthnCard(props) {
const onSubmitKeyName = async (values) => {
let response;
try {
response = await SeaCatAuthAPI.put(`/public/webauthn/${values.id}`,
response = await SeaCatAccountAPI.put(`/webauthn/${values.id}`,
{"name": `${values.name}`}
);
if (response.data.result != 'OK') {
Expand Down