Impossible Travel Detection

This implements a very simple variant of an impossible travel detection

Implementation Overview

The impossible travel detection is implemented as a PostLoginHook in selfservice/hook/impossible_travel.go.

It uses the IP address currently used for the login (extracted from the request) and compares it to IP addresses of the devices belonging to the most recent created sessions. The IP addresses are passed onto getLocationForIpAddress which could do API calls to any geolocalisation service (but in this implementation is just mocked to return three static locations depending on the given IP address).

For two locations, a distance is calculated using simple linear algebra and trigonometry (MinDistanceKm). In combination with the devices updated_at timestamps and the configured maximum travel speed (currently just fixed to 1000 km/h), we can calculate whether the login looks like the user has been traveling impossibly fast.

If the login looks like the user has been traveling impossibly fast an error is returned to the users. (See notes below.)

Tests in selfservice/hook/impossible_travel_test.go test the functionality of the hook similarly to the tests in selfservice/hook/require_verified_address_test.go. Also the MinDistanceKm function is tested.

Assumptions

• Session objects are only created and persisted after a successful login. If unsuccessful logins can lead to Session objects being persisted, we would need to filter for sessions belonging to actually successful logins, because otherwise an attacker might make it impossible for a valid user to login be creating unsuccessful logins far away from their location. The fact, that our ExecuteLoginPostHook get's a Session object gives a hint that this might actually be the case.

• A sessions created_at and it's devices last usage timestamps are strongly correlated. If they might not be, it might be better to not use the most recently created sessions, but the most recently used sessions. However ListSessionsByIdentity only returns the most recently created sessions. So we could either fetch more sessions and sort them by their last usage value or add a new query functions to get the sessions ordered by their last usage.

Whats missing

• The setting that impossible travel detection should be applied ond a maximal travel speed should be configurable. This can probably be done by adding the corresponding configuration values to driver/config/config.go and using these values in the impossible travel detection hook.

• It is probably much better, to not have a single getLocationForIpAddress function, but to allow for different implementations to be used. This would allow to use different APIs for geolocalisation and have a mocked implementation for testing purposes. This would probably be done by creating a GeoLocalisationProvider interface, which could then be injected into the impossible travel hook in the same way that the PersistenceProvider is currently injected.

• In the implementation, I'm currently using the devices' created_at timestamp instead of the updated_at timestamps. Using updated_at is much more sensible, but because the tests use reg.SessionPersister().UpsertSession() to insert the sessions into the database and this function updates the devices' updated_at timestamps, the tests are currently broken for updated_at. One fix for this would be to manually update the device's updated_at timestamp after inserting the session into the database.

Other Notes

• I have calculated all distances in km, all durations in hours and thus all speeds in km/h, as they are the most natural to me. But depending on the conventions, this could of course also be done in miles and mph or (and this is what I would probably prefer in real production code) in SI units (i.e. meters, seconds and m/s), however this is probably not the way you want users to configure this, so you would have to convert between user provided units (km/h or mph) to SI units (m/s). (This would also simplify variable names a lot.)

• Currently the error message that the maximum travel speed has been exceeded is shown to the user. Because this happens when we suspect illigitimate access, we probably do not want to let the attacker know WHY their loginattempt has been unsuccessful. Instead we should show the a more generic error message.