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

Fix CSRF token issues #1057

Merged
merged 4 commits into from
Aug 6, 2024
Merged
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
2 changes: 1 addition & 1 deletion backend/endpoints/heartbeat.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

@router.get("/heartbeat")
def heartbeat() -> HeartbeatResponse:
"""Endpoint to set the CSFR token in cache and return all the basic RomM config
"""Endpoint to set the CSRF token in cache and return all the basic RomM config

Returns:
HeartbeatReturn: TypedDict structure with all the defined values in the HeartbeatReturn class.
Expand Down
1 change: 1 addition & 0 deletions backend/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]:
# CSRF protection (except endpoints listed in exempt_urls)
app.add_middleware(
CustomCSRFMiddleware,
cookie_name="romm_csrftoken",
secret=ROMM_AUTH_SECRET_KEY,
exempt_urls=[re.compile(r"^/token.*"), re.compile(r"^/ws")],
)
Expand Down
52 changes: 18 additions & 34 deletions frontend/src/App.vue
Original file line number Diff line number Diff line change
Expand Up @@ -5,50 +5,34 @@ import userApi from "@/services/api/user";
import storeAuth from "@/stores/auth";
import storeConfig from "@/stores/config";
import storeHeartbeat from "@/stores/heartbeat";
import storeNotifications from "@/stores/notifications";
import Cookies from "js-cookie";
import { storeToRefs } from "pinia";
import { onBeforeMount } from "vue";
import router from "./plugins/router";

// Props
const notificationStore = storeNotifications();
const { notifications } = storeToRefs(notificationStore);
const heartbeat = storeHeartbeat();
const auth = storeAuth();
const configStore = storeConfig();

onBeforeMount(async () => {
await api
.get("/heartbeat")
.then(async ({ data: data }) => {
heartbeat.set(data);
if (heartbeat.value.SHOW_SETUP_WIZARD) {
router.push({ name: "setup" });
} else {
await userApi
.fetchCurrentUser()
.then(({ data: user }) => {
auth.setUser(user);
})
.catch((error) => {
console.error(error);
});

await api.get("/config").then(({ data: data }) => {
configStore.set(data);
onBeforeMount(() => {
api.get("/heartbeat").then(async ({ data: data }) => {
heartbeat.set(data);
if (heartbeat.value.SHOW_SETUP_WIZARD) {
router.push({ name: "setup" });
} else {
await userApi
.fetchCurrentUser()
.then(({ data: user }) => {
auth.setUser(user);
})
.catch((error) => {
console.error(error);
});
}
})
.catch(() => {
const allCookies = Cookies.get(); // Get all cookies
for (let cookie in allCookies) {
Cookies.remove(cookie); // Remove each cookie
}
router.push({
name: "login",

await api.get("/config").then(({ data: data }) => {
configStore.set(data);
});
});
}
});
});
</script>

Expand Down
7 changes: 6 additions & 1 deletion frontend/src/components/common/Navigation/SettingsDrawer.vue
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<script setup lang="ts">
import type { UserSchema } from "@/__generated__";
import identityApi from "@/services/api/identity";
import { refetchCSRFToken } from "@/services/api/index";
import storeAuth from "@/stores/auth";
import storeNavigation from "@/stores/navigation";
import type { Events } from "@/types/emitter";
Expand All @@ -22,13 +23,17 @@ const { smAndDown } = useDisplay();

// Functions
async function logout() {
identityApi.logout().then(({ data }) => {
identityApi.logout().then(async ({ data }) => {
// Refetch CSRF token
await refetchCSRFToken();
Copy link
Member

Choose a reason for hiding this comment

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

Reset the CSRF token and logout and fetch a new one


emitter?.emit("snackbarShow", {
msg: data.msg,
icon: "mdi-check-bold",
color: "green",
});
});

await router.push({ name: "login" });
auth.setUser(null);
}
Expand Down
29 changes: 21 additions & 8 deletions frontend/src/services/api/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import router from "@/plugins/router";
import axios from "axios";
import { default as cookie, default as Cookies } from "js-cookie";
import { default as Cookies } from "js-cookie";
import { debounce } from "lodash";

const api = axios.create({ baseURL: "/api", timeout: 120000 });
Expand All @@ -19,7 +19,7 @@ api.interceptors.request.use((config) => {
networkQuiesced.cancel();

// Set CSRF header for all requests
config.headers["x-csrftoken"] = cookie.get("csrftoken");
config.headers["x-csrftoken"] = Cookies.get("romm_csrftoken");
return config;
});

Expand All @@ -35,19 +35,32 @@ api.interceptors.response.use(

return response;
},
(error) => {
async (error) => {
if (error.response?.status === 403) {
const allCookies = Cookies.get(); // Get all cookies
for (const cookie in allCookies) {
Cookies.remove(cookie); // Remove each cookie
}
// Clear cookies and redirect to login page
Cookies.remove("romm_session");

// Refetch CSRF cookie
await refetchCSRFToken();

const pathname = window.location.pathname;
const params = new URLSearchParams(window.location.search);

router.push({
name: "login",
query: { next: router.currentRoute.value.path },
query: {
next: params.get("next") ?? (pathname !== "/login" ? pathname : "/"),
},
});
}
return Promise.reject(error);
},
);

export default api;

export async function refetchCSRFToken() {
Cookies.remove("romm_csrftoken");

return await api.get("/heartbeat");
}
16 changes: 11 additions & 5 deletions frontend/src/views/Login.vue
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<script setup lang="ts">
import identityApi from "@/services/api/identity";
import { refetchCSRFToken } from "@/services/api/index";
import storeHeartbeat from "@/stores/heartbeat";
import type { Events } from "@/types/emitter";
import type { Emitter } from "mitt";
Expand All @@ -17,11 +18,15 @@ const logging = ref(false);

async function login() {
logging.value = true;

await identityApi
.login(username.value, password.value)
.then(() => {
const next = (router.currentRoute.value.query?.next || "/").toString();
router.push(next);
.then(async () => {
// Refetch CSRF token
await refetchCSRFToken();
Copy link
Member

Choose a reason for hiding this comment

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

Same here but after login (new one will be fetched as well)


const params = new URLSearchParams(window.location.search);
router.push(params.get("next") ?? "/");
})
.catch(({ response, message }) => {
const errorMessage =
Expand All @@ -35,6 +40,7 @@ async function login() {
icon: "mdi-close-circle",
color: "red",
});

console.error(
`[${response.status} ${response.statusText}] ${errorMessage}`
);
Expand All @@ -59,15 +65,16 @@ async function login() {
<v-form @submit.prevent>
<v-text-field
v-model="username"
autocomplete="on"
required
prepend-inner-icon="mdi-account"
type="text"
label="Username"
variant="underlined"
@keyup.enter="login()"
Copy link
Member

Choose a reason for hiding this comment

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

don't need these since you're using a password field

/>
<v-text-field
v-model="password"
autocomplete="on"
required
prepend-inner-icon="mdi-lock"
:type="visiblePassword ? 'text' : 'password'"
Expand All @@ -76,7 +83,6 @@ async function login() {
:append-inner-icon="
visiblePassword ? 'mdi-eye-off' : 'mdi-eye'
"
@keyup.enter="login()"
@click:append-inner="visiblePassword = !visiblePassword"
/>
<v-btn
Expand Down
7 changes: 3 additions & 4 deletions frontend/src/views/Setup.vue
Original file line number Diff line number Diff line change
@@ -1,18 +1,17 @@
<script setup lang="ts">
import router from "@/plugins/router";
import { refetchCSRFToken } from "@/services/api/index";
import userApi from "@/services/api/user";
import storeHeartbeat from "@/stores/heartbeat";
import type { Events } from "@/types/emitter";
import type { Emitter } from "mitt";
import { useRoute } from "vue-router";
import { computed, inject, ref } from "vue";
import { useDisplay } from "vuetify";

// Props
const { xs, smAndDown } = useDisplay();
const emitter = inject<Emitter<Events>>("emitter");
const heartbeat = storeHeartbeat();
const route = useRoute()
const visiblePassword = ref(false);
// Use a computed property to reactively update metadataOptions based on heartbeat
const metadataOptions = computed(() => [
Expand Down Expand Up @@ -53,8 +52,8 @@ const isLastStep = computed(() => step.value == 2);
async function finishWizard() {
await userApi
.createUser(defaultAdminUser.value)
.then(() => {
router.go(0); // Needed to get the csrftoken properly after creating default admin user
.then(async () => {
await refetchCSRFToken();
router.push({ name: "login" });
})
.catch(({ response, message }) => {
Expand Down
Loading