Skip to content
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
10 changes: 10 additions & 0 deletions backend/src/appointment/exceptions/validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -364,3 +364,13 @@ def get_msg(self):

def get_reason(self):
return l10n('google-caldav-not-supported-details')


class ConnectionContainsDefaultCalendarException(APIException):
"""Raise when attempting to disconnect an external connection that contains the default calendar"""

id_code = 'CONNECTION_CONTAINS_DEFAULT_CALENDAR'
status_code = 400

def get_msg(self):
return l10n('connection-contains-default-calendar')
1 change: 1 addition & 0 deletions backend/src/appointment/l10n/de/main.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ schedule-availability-invalid = Die benutzerdefinierte Verfügbarkeit ist nicht
schedule-not-active = Der Zeitplan wurde abgeschaltet. Bitte für weitere Informationen den Eigentümer des Zeitplans kontaktieren.

remote-calendar-connection-error = Es konnte keine Verbindung hergestellt werden.
connection-contains-default-calendar = Dieses Konto kann nicht getrennt werden, da es den Standardkalender enthält. Bitte zuerst einen anderen Kalender als Standard festlegen.

# Possible error reasons
remote-calendar-reason-doesnt-support-caldav = Der Kalender bietet keine CalDAV-Unterstützung.
Expand Down
1 change: 1 addition & 0 deletions backend/src/appointment/l10n/en/main.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ schedule-availability-invalid = The custom availability is not valid. Please che
schedule-not-active = The schedule has been turned off. Please contact the schedule owner for more information.

remote-calendar-connection-error = We couldn't connect to your calendar.
connection-contains-default-calendar = Cannot disconnect this account because it contains your default calendar. Please set a different calendar as default first.

# Possible error reasons
remote-calendar-reason-doesnt-support-caldav = The remote calendar does not support CalDAV.
Expand Down
15 changes: 13 additions & 2 deletions backend/src/appointment/routes/caldav.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@
from appointment.dependencies.database import get_db, get_redis
from appointment.exceptions.calendar import TestConnectionFailed
from appointment.exceptions.misc import UnexpectedBehaviourWarning
from appointment.exceptions.validation import RemoteCalendarConnectionError, GoogleCaldavNotSupported
from appointment.exceptions.validation import (
RemoteCalendarConnectionError,
GoogleCaldavNotSupported,
ConnectionContainsDefaultCalendarException,
)
from appointment.l10n import l10n
from appointment.defines import GOOGLE_CALDAV_DOMAINS

Expand Down Expand Up @@ -173,7 +177,14 @@ def disconnect_account(
)

if ec is None:
return RemoteCalendarConnectionError()
raise RemoteCalendarConnectionError()

# Check if any schedule uses a calendar from this connection as its default
# If so, prevent disconnection to avoid breaking the user's booking setup
schedules = repo.schedule.get_by_subscriber(db, subscriber.id)
for schedule in schedules:
if schedule.calendar and schedule.calendar.external_connection_id == ec.id:
raise ConnectionContainsDefaultCalendarException()

# Deserialize the url/user
_, user = json.loads(ec.type_id)
Expand Down
9 changes: 8 additions & 1 deletion backend/src/appointment/routes/google.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from ..dependencies.google import get_google_client
from ..exceptions.google_api import GoogleInvalidCredentials
from ..exceptions.google_api import GoogleScopeChanged
from ..exceptions.validation import OAuthFlowNotFinished
from ..exceptions.validation import OAuthFlowNotFinished, ConnectionContainsDefaultCalendarException
from ..l10n import l10n

router = APIRouter()
Expand Down Expand Up @@ -155,6 +155,13 @@ def disconnect_account(
"""Disconnects a google account. Removes associated data from our services and deletes the connection details."""
google_connection = subscriber.get_external_connection(ExternalConnectionType.google, request_body.type_id)

# Check if any schedule uses a calendar from this connection as its default
# If so, prevent disconnection to avoid breaking the user's booking setup
schedules = repo.schedule.get_by_subscriber(db, subscriber.id)
for schedule in schedules:
if schedule.calendar and schedule.calendar.external_connection_id == google_connection.id:
raise ConnectionContainsDefaultCalendarException()

# Remove all of the google calendars on their given google connection
repo.calendar.delete_by_subscriber_and_provider(
db, subscriber.id, provider=models.CalendarProvider.google, external_connection_id=google_connection.id
Expand Down
135 changes: 135 additions & 0 deletions backend/test/integration/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -581,6 +581,79 @@ def test_disconnect(self, with_db, with_client, make_external_connections, make_
calendar = repo.calendar.get(db, calendar.id)
assert calendar is None

def test_disconnect_fails_when_connection_contains_default_calendar(
self, with_db, with_client, make_external_connections, make_caldav_calendar, make_schedule
):
"""Ensure we cannot disconnect an external connection that contains the default calendar (used by a schedule)"""
username = 'username'
type_id = json.dumps(['url', username])
ec = make_external_connections(TEST_USER_ID, type=models.ExternalConnectionType.caldav, type_id=type_id)
calendar = make_caldav_calendar(
subscriber_id=TEST_USER_ID, user=username, connected=True, external_connection_id=ec.id
)

# Create a schedule that uses this calendar (making it the "default" calendar)
make_schedule(calendar_id=calendar.id)

# Attempt to disconnect - should fail
# Note: type_id is a query parameter in the caldav disconnect route, not a body parameter
response = with_client.post('/caldav/disconnect', params={'type_id': ec.type_id}, headers=auth_headers)

assert response.status_code == 400, response.content
data = response.json()
assert data['detail']['id'] == 'CONNECTION_CONTAINS_DEFAULT_CALENDAR'

# Verify the connection and calendar still exist
with with_db() as db:
ecs = repo.external_connection.get_by_type(
db, TEST_USER_ID, models.ExternalConnectionType.caldav, type_id=type_id
)
assert len(ecs) == 1

calendar_check = repo.calendar.get(db, calendar.id)
assert calendar_check is not None

def test_disconnect_succeeds_when_different_connection_contains_default_calendar(
self, with_db, with_client, make_external_connections, make_caldav_calendar, make_schedule
):
"""Ensure we can disconnect an external connection when a different connection contains the default calendar"""
# Create first connection with calendar that will be the default
username1 = 'username1'
type_id1 = json.dumps(['url1', username1])
ec1 = make_external_connections(TEST_USER_ID, type=models.ExternalConnectionType.caldav, type_id=type_id1)
calendar1 = make_caldav_calendar(
subscriber_id=TEST_USER_ID, user=username1, connected=True, external_connection_id=ec1.id
)

# Create second connection with calendar that we'll try to disconnect
username2 = 'username2'
type_id2 = json.dumps(['url2', username2])
ec2 = make_external_connections(TEST_USER_ID, type=models.ExternalConnectionType.caldav, type_id=type_id2)
make_caldav_calendar(
subscriber_id=TEST_USER_ID, user=username2, connected=True, external_connection_id=ec2.id
)

# Create a schedule that uses the first calendar (making it the "default" calendar)
make_schedule(calendar_id=calendar1.id)

# Attempt to disconnect the second connection - should succeed
# Note: type_id is a query parameter in the caldav disconnect route, not a body parameter
response = with_client.post('/caldav/disconnect', params={'type_id': ec2.type_id}, headers=auth_headers)

assert response.status_code == 200, response.content

# Verify the second connection is removed but the first still exists
with with_db() as db:
ecs1 = repo.external_connection.get_by_type(
db, TEST_USER_ID, models.ExternalConnectionType.caldav, type_id=type_id1
)
assert len(ecs1) == 1

ecs2 = repo.external_connection.get_by_type(
db, TEST_USER_ID, models.ExternalConnectionType.caldav, type_id=type_id2
)
assert len(ecs2) == 0


class TestGoogle:
def test_disconnect(self, with_db, with_client, make_external_connections, make_google_calendar):
Expand Down Expand Up @@ -656,6 +729,68 @@ def test_disconnect_with_multiple_accounts(
all_ecs = repo.external_connection.get_by_type(db, TEST_USER_ID, models.ExternalConnectionType.google)
assert len(all_ecs) == 1

def test_disconnect_fails_when_connection_contains_default_calendar(
self, with_db, with_client, make_external_connections, make_google_calendar, make_schedule
):
"""Ensure we cannot disconnect an external connection that contains the default calendar (used by a schedule)"""
type_id = str(uuid4())
ec = make_external_connections(TEST_USER_ID, type=models.ExternalConnectionType.google, type_id=type_id)
calendar = make_google_calendar(subscriber_id=TEST_USER_ID, connected=True, external_connection_id=ec.id)

# Create a schedule that uses this calendar (making it the "default" calendar)
make_schedule(calendar_id=calendar.id)

# Attempt to disconnect - should fail
response = with_client.post('/google/disconnect', json={'type_id': ec.type_id}, headers=auth_headers)

assert response.status_code == 400, response.content
data = response.json()
assert data['detail']['id'] == 'CONNECTION_CONTAINS_DEFAULT_CALENDAR'

# Verify the connection and calendar still exist
with with_db() as db:
ecs = repo.external_connection.get_by_type(
db, TEST_USER_ID, models.ExternalConnectionType.google, type_id=type_id
)
assert len(ecs) == 1

calendar_check = repo.calendar.get(db, calendar.id)
assert calendar_check is not None

def test_disconnect_succeeds_when_different_connection_contains_default_calendar(
self, with_db, with_client, make_external_connections, make_google_calendar, make_schedule
):
"""Ensure we can disconnect an external connection when a different connection contains the default calendar"""
# Create first connection with calendar that will be the default
type_id1 = str(uuid4())
ec1 = make_external_connections(TEST_USER_ID, type=models.ExternalConnectionType.google, type_id=type_id1)
calendar1 = make_google_calendar(subscriber_id=TEST_USER_ID, connected=True, external_connection_id=ec1.id)

# Create second connection with calendar that we'll try to disconnect
type_id2 = str(uuid4())
ec2 = make_external_connections(TEST_USER_ID, type=models.ExternalConnectionType.google, type_id=type_id2)
make_google_calendar(subscriber_id=TEST_USER_ID, connected=True, external_connection_id=ec2.id)

# Create a schedule that uses the first calendar (making it the "default" calendar)
make_schedule(calendar_id=calendar1.id)

# Attempt to disconnect the second connection - should succeed
response = with_client.post('/google/disconnect', json={'type_id': ec2.type_id}, headers=auth_headers)

assert response.status_code == 200, response.content

# Verify the second connection is removed but the first still exists
with with_db() as db:
ecs1 = repo.external_connection.get_by_type(
db, TEST_USER_ID, models.ExternalConnectionType.google, type_id=type_id1
)
assert len(ecs1) == 1

ecs2 = repo.external_connection.get_by_type(
db, TEST_USER_ID, models.ExternalConnectionType.google, type_id=type_id2
)
assert len(ecs2) == 0

def test_sync_calendars(self, with_db, with_client, make_external_connections, monkeypatch):
"""Test that sync_calendars creates calendars from Google API response"""
from appointment.controller.apis.google_client import GoogleClient
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,22 @@ const availabilityStore = useAvailabilityStore();
const { currentState } = storeToRefs(settingsStore);
const { calendars } = storeToRefs(calendarStore);

const calendarConnectedMap = new Map<number, ReturnType<typeof computed<boolean>>>();

const zoomAccount = computed(() => externalConnectionStore.zoom[0]);

// This computed property refers to the calendars from the backend combined
// with some data from external connections to facilitate connection / disconnection.
// It should be immutable as it represents the current state of the backend data.
const initialCalendars = computed(() => {
// Find the default calendar's external_connection_id to determine which
// calendars share the same connection. Calendars sharing the default's
// connection cannot be disconnected without affecting the default calendar.
const defaultCalendar = calendars.value?.find(
Copy link
Member

Choose a reason for hiding this comment

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

I feel like this needs to be a backend check as well to avoid validation bypass.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call!

Added in this same PR here for completeness:

This commit adds the guards in Google and CalDAV /disconnect routes: 1c36cb7

This commit adds the tests: c0b29fb

(Note to self: a little weird that CalDAV gets the type_id as query param and Google gets it as body 🤔 perhaps a change for another time heh)

(calendar) => calendar.id === currentState.value.defaultCalendarId
);
const defaultExternalConnectionId = defaultCalendar?.external_connection_id;

const formattedCalendars = calendars.value?.map((calendar) => {
const connectionProvider = keyByValue(CalendarProviders, calendar.provider, true);
const externalConnection: ExternalConnection = externalConnectionStore
Expand All @@ -60,6 +70,7 @@ const initialCalendars = computed(() => {
connection_name: externalConnection?.name,
provider_name: connectionProvider,
is_default: currentState.value.defaultCalendarId === calendar.id,
shares_default_connection: calendar.external_connection_id === defaultExternalConnectionId,
}
}) || [];

Expand Down Expand Up @@ -116,17 +127,29 @@ function closeModals() {
disconnectConnectionName.value = null;
}

function onCalendarChecked(event: HTMLInputElementEvent, calendarId: number) {
// Only update local state, the actual connection / disconnection
// of the calendar happens on save changes in SettingsView/index.vue
settingsStore.$patch({
currentState: {
changedCalendars: {
...currentState.value.changedCalendars,
[calendarId]: event.target.checked
function calendarConnected(calendarId: number) {
if (!calendarConnectedMap.has(calendarId)) {
calendarConnectedMap.set(calendarId, computed({
get: () => {
const calendar = initialCalendars.value.find(c => c.id === calendarId);
return currentState.value.changedCalendars?.[calendarId] !== undefined
? currentState.value.changedCalendars[calendarId]
: calendar?.connected ?? false;
},
set: (value: boolean) => {
settingsStore.$patch({
currentState: {
changedCalendars: {
...currentState.value.changedCalendars,
[calendarId]: value
}
}
})
}
}
})
}));
}

return calendarConnectedMap.get(calendarId)!;
}

function onSetAsDefaultClicked(calendarId: number) {
Expand Down Expand Up @@ -230,8 +253,7 @@ async function refreshData() {
<checkbox-input
:name="`calendarConnected-${calendar.id}`"
class="calendar-connected-checkbox"
@change="(event) => onCalendarChecked(event, calendar.id)"
:checked="currentState.changedCalendars?.[calendar.id] !== undefined ? currentState.changedCalendars[calendar.id] : calendar.connected"
v-model="calendarConnected(calendar.id).value"
:disabled="calendar.is_default"
/>

Expand Down Expand Up @@ -261,18 +283,20 @@ async function refreshData() {
<p class="calendar-provider">{{ calendar.provider_name }}</p>

<!-- Dropdown actions -->
<!-- Hide dropdown for calendars that share the same ExternalConnection as the default calendar,
since disconnecting would affect the default calendar -->
<drop-down
v-if="!calendar.is_default"
v-if="!calendar.shares_default_connection"
class="dropdown"
:ref="(el) => calendarDropdownRefs[calendar.id] = el"
>
<template #trigger>
<ph-dots-three size="24" />
</template>
<template #default>
<div class="dropdown-inner" @click="calendarDropdownRefs[calendar.id].close()">
<div class="dropdown-inner" @click="calendarDropdownRefs[calendar.id]?.close()">
<button
v-if="calendar.connected && !calendar.is_default"
v-if="calendar.connected"
@click="() => onSetAsDefaultClicked(calendar.id)"
>
{{ t('text.settings.connectedApplications.setAsDefault') }}
Expand All @@ -283,7 +307,6 @@ async function refreshData() {
</button> -->
<button
@click="() => displayModal(calendar.provider, calendar.type_id, calendar.connection_name, true)"
:disabled="calendar.is_default"
>
{{ t('label.disconnect') }}
</button>
Expand Down
10 changes: 10 additions & 0 deletions test/e2e/pages/settings-page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ export class SettingsPage {
readonly addCaldavCloseModalBtn: Locator;
readonly addGoogleBtn: Locator;
readonly defaultCalendarBadge: Locator;
readonly calendarDropdownTriggers: Locator;
readonly calendarDropdownSetAsDefault: Locator;
readonly calendarDropdownDisconnect: Locator;
readonly calendarCheckboxes: Locator;
readonly unsavedChangesNotice: Locator;
readonly saveBtnEN: Locator;
readonly savedSuccessfullyTextEN: Locator;
readonly savedSuccessfullyTextDE: Locator;
Expand Down Expand Up @@ -96,6 +101,11 @@ export class SettingsPage {
this.addCaldavCloseModalBtn = this.page.getByRole('img', { name: 'Close' });
this.addGoogleBtn = this.page.getByRole('button', { name: 'Add Google Calendar' });
this.defaultCalendarBadge = this.page.getByTestId('badge');
this.calendarDropdownTriggers = this.page.locator('.calendars-container .dropdown');
this.calendarDropdownSetAsDefault = this.page.getByRole('button', { name: 'Set as default' });
this.calendarDropdownDisconnect = this.page.locator('.calendars-container .dropdown-inner').getByRole('button', { name: 'Disconnect' });
this.calendarCheckboxes = this.page.locator('.calendars-container input[type="checkbox"]');
this.unsavedChangesNotice = this.page.getByText('You have unsaved changes');
this.googleSignInHdr = this.page.getByText('Sign in with Google');
}

Expand Down
Loading