Redirect to the login view when login is required

When the `UMAP_ALLOW_ANONYMOUS` setting is False, we return the login url through a JSON response. We lost that ability in #1555.

The JS part was not following that link in that particular case and lead to more errors because the map was not saved (hence, no `map_id`).

For now, the current work on the map is lost because of the redirection and we have a confirmation dialog to quit the edited page with unsaved changes.

Maybe we should display a custom message instead of a brutal redirection? Like: you’re not logged in, do it in a separate tab to keep you work? (A bit ugly…)

Sidenote: we might want to use the `redirect` pattern/key in the JSON response that we already use for deletion and clone for consistency.
This commit is contained in:
David Larlet 2024-04-05 13:17:58 -04:00
parent 99a0007ce0
commit 0ec3816f93
No known key found for this signature in database
GPG key ID: 3E2953A359E7E7BD
4 changed files with 17 additions and 12 deletions

View file

@ -19,7 +19,9 @@ def login_required_if_not_anonymous_allowed(view_func):
not getattr(settings, "UMAP_ALLOW_ANONYMOUS", False) not getattr(settings, "UMAP_ALLOW_ANONYMOUS", False)
and not request.user.is_authenticated and not request.user.is_authenticated
): ):
return simple_json_response(login_required=str(LOGIN_URL)) response = simple_json_response(login_required=str(LOGIN_URL))
response.status_code = 401
return response
return view_func(request, *args, **kwargs) return view_func(request, *args, **kwargs)
return wrapper return wrapper
@ -39,7 +41,9 @@ def can_edit_map(view_func):
can_edit = map_inst.can_edit(user=user, request=request) can_edit = map_inst.can_edit(user=user, request=request)
if not can_edit: if not can_edit:
if map_inst.owner and not user.is_authenticated: if map_inst.owner and not user.is_authenticated:
return simple_json_response(login_required=str(LOGIN_URL)) response = simple_json_response(login_required=str(LOGIN_URL))
response.status_code = 401
return response
return HttpResponseForbidden() return HttpResponseForbidden()
return view_func(request, *args, **kwargs) return view_func(request, *args, **kwargs)

View file

@ -76,7 +76,6 @@ U.Map = L.Map.extend({
.split(',') .split(',')
} }
let editedFeature = null let editedFeature = null
const self = this const self = this
try { try {
@ -962,8 +961,10 @@ U.Map = L.Map.extend({
formData.append('settings', JSON.stringify(geojson)) formData.append('settings', JSON.stringify(geojson))
const uri = this.urls.get('map_save', { map_id: this.options.umap_id }) const uri = this.urls.get('map_save', { map_id: this.options.umap_id })
const [data, response, error] = await this.server.post(uri, {}, formData) const [data, response, error] = await this.server.post(uri, {}, formData)
// FIXME: login_required response will not be an error, so it will not if (error && response.status === 401) {
// stop code while it should const data = await response.json()
window.location = data.login_required
}
if (!error) { if (!error) {
let duration = 3000, let duration = 3000,
alert = { content: L._('Map has been saved!'), level: 'info' } alert = { content: L._('Map has been saved!'), level: 'info' }

View file

@ -134,7 +134,7 @@ class DataLayerFactory(factory.django.DjangoModelFactory):
def login_required(response): def login_required(response):
assert response.status_code == 200 assert response.status_code == 401
j = json.loads(response.content.decode()) j = json.loads(response.content.decode())
assert "login_required" in j assert "login_required" in j
redirect_url = reverse("login") redirect_url = reverse("login")

View file

@ -608,7 +608,7 @@ def test_cannot_send_link_on_owned_map(client, map):
assert len(mail.outbox) == 0 assert len(mail.outbox) == 0
url = reverse("map_send_edit_link", args=(map.pk,)) url = reverse("map_send_edit_link", args=(map.pk,))
resp = client.post(url, {"email": "foo@bar.org"}) resp = client.post(url, {"email": "foo@bar.org"})
assert resp.status_code == 200 assert resp.status_code == 401
assert json.loads(resp.content.decode()) == {"login_required": "/en/login/"} assert json.loads(resp.content.decode()) == {"login_required": "/en/login/"}
assert len(mail.outbox) == 0 assert len(mail.outbox) == 0