Quick Add + Update Cryptography for API Key Generation#1099
Quick Add + Update Cryptography for API Key Generation#1099seanmorley15 wants to merge 31 commits intomainfrom
Conversation
…ng and encapsulate comment and close logic
- Implemented API key creation, deletion, and display functionality. - Updated the settings page to fetch and show existing API keys. - Added UI elements for creating new API keys and copying them to clipboard. - Enhanced request handling to ensure proper trailing slashes for API endpoints.
- dompurify: upgraded from 3.3.1 to 3.3.3 - emoji-picker-element: upgraded from 1.29.0 to 1.29.1 - @sveltejs/adapter-node: updated to use @sveltejs/kit@2.55.0 - @sveltejs/adapter-vercel: updated to use @sveltejs/kit@2.55.0 - @sveltejs/kit: upgraded from 2.53.3 to 2.55.0 - @types/node: upgraded from 22.19.13 to 22.19.15 - autoprefixer: updated postcss version from 8.5.6 to 8.5.8 - baseline-browser-mapping: upgraded from 2.10.0 to 2.10.8 - daisyui: updated postcss version from 8.5.6 to 8.5.8 - prettier-plugin-svelte: upgraded from 3.5.0 to 3.5.1 - svelte-check: updated postcss version from 8.5.6 to 8.5.8 - devalue: upgraded from 5.6.3 to 5.6.4 - electron-to-chromium: upgraded from 1.5.302 to 1.5.313 - caniuse-lite: upgraded from 1.0.30001774 to 1.0.30001780 - mlly: upgraded from 1.8.0 to 1.8.1 - node-releases: upgraded from 2.0.27 to 2.0.36 - tar: upgraded from 7.5.9 to 7.5.11 - tinyexec: upgraded from 1.0.2 to 1.0.4
Currently translated at 99.9% (1091 of 1092 strings) Translation: AdventureLog/Web App Translate-URL: https://hosted.weblate.org/projects/adventurelog/web-app/fr/
Currently translated at 100.0% (1092 of 1092 strings) Translation: AdventureLog/Web App Translate-URL: https://hosted.weblate.org/projects/adventurelog/web-app/ko/
Currently translated at 100.0% (1092 of 1092 strings) Translation: AdventureLog/Web App Translate-URL: https://hosted.weblate.org/projects/adventurelog/web-app/de/
Currently translated at 100.0% (1092 of 1092 strings) Translation: AdventureLog/Web App Translate-URL: https://hosted.weblate.org/projects/adventurelog/web-app/sv/
Currently translated at 1.2% (14 of 1092 strings) Translation: AdventureLog/Web App Translate-URL: https://hosted.weblate.org/projects/adventurelog/web-app/ca/
* Refactor AdventureLog Bot workflow to improve issue validation handling and encapsulate comment and close logic (#1068) * Reorder immich API permissions to natural order --------- Co-authored-by: Sean Morley <git@seanmorley.com>
Currently translated at 100.0% (1093 of 1093 strings) Translation: AdventureLog/Web App Translate-URL: https://hosted.weblate.org/projects/adventurelog/web-app/tr/
Currently translated at 100.0% (1093 of 1093 strings) Translation: AdventureLog/Web App Translate-URL: https://hosted.weblate.org/projects/adventurelog/web-app/sv/
Currently translated at 100.0% (1093 of 1093 strings) Translation: AdventureLog/Web App Translate-URL: https://hosted.weblate.org/projects/adventurelog/web-app/de/
…ty and location enrichment - Added quick add feature for locations with category selection. - Implemented location description enrichment using Google Maps API. - Improved search functionality and result handling. - Introduced new utility functions for location saving and validation. - Updated UI to reflect changes in location selection and quick add status. - Added toast notifications for user feedback on actions. - Refactored existing code for better readability and maintainability. fix: Ensure finite coordinates in LocationSearchMap component - Added validation for initial selection coordinates to prevent errors. chore: Update app version to v0.12.0-main-033126 - Updated versioning in config file. feat: Create location-save module for handling location data saving - Implemented saveLocation function to handle both new and existing location data. - Added utility functions for coordinate formatting and link sanitization.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR adds a streamlined “quick add” location flow and substantially expands backend geocoding + remote image import capabilities, while also updating API key hashing and some frontend location-editing behavior.
Changes:
- Added backend endpoints/utilities for quick-adding locations, enriching place details, and bulk importing remote images with SSRF/redirect validation and parallel downloads.
- Enhanced geocoding to support richer Google Place results (types/ratings/photos) and added a place-details enrichment path (Google + Wikipedia).
- Refactored frontend location creation/editing flow to support quick-start selection, quick add, and improved save payload normalization.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 25 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/locales/pl.json | Updates Polish translations (currently removes several keys). |
| frontend/src/locales/no.json | Updates Norwegian translations (currently removes several keys). |
| frontend/src/locales/nl.json | Updates Dutch translations (currently removes several keys). |
| frontend/src/locales/ko.json | Updates Korean translations (currently removes several keys / tweaks wording). |
| frontend/src/locales/ja.json | Updates Japanese translations (currently removes several keys). |
| frontend/src/lib/location-save.ts | Adds a shared location save helper (payload cleanup/normalization + error parsing). |
| frontend/src/lib/components/shared/LocationSearchMap.svelte | Improves initial selection validation and name/display-name resolution behavior. |
| frontend/src/lib/components/locations/LocationQuickStart.svelte | Adds richer place selection + “Quick Add” UX with enrichment hooks. |
| frontend/src/lib/components/locations/LocationModal.svelte | Integrates quick-start/quick-add flow and optional background import of Google photos. |
| frontend/src/lib/components/locations/LocationDetails.svelte | Refactors save logic to use the shared save helper and improves lat/lng coercion. |
| CONTRIBUTING.md | Updates docs path reference in contributing guide. |
| backend/server/users/models.py | Changes API key hashing from SHA-256 to PBKDF2-HMAC-SHA256. |
| backend/server/templates/base.html | Fixes a JS function name typo in the template. |
| backend/server/adventures/views/reverse_geocode_view.py | Adds a place_details endpoint to return enriched place details. |
| backend/server/adventures/views/location_view.py | Adds quick-add endpoint and helper coercion/normalization methods. |
| backend/server/adventures/views/location_image_view.py | Adds remote image download/import utilities and a bulk import endpoint. |
| backend/server/adventures/geocoding.py | Expands Google search payload/results + adds place-details enrichment (Google + Wikipedia) and improved reverse-geocode naming. |
| .github/workflows/adventurelog-bot.yml | Expands PR-actor skip logic (maintainer + dependabot). |
| @@ -28,8 +28,7 @@ | |||
| "calendar": "Kalendarz", | |||
| "admin_panel": "Panel administracyjny", | |||
| "navigation": "Nawigacja", | |||
There was a problem hiding this comment.
The navbar.mobile_login translation key was removed, but it’s still referenced in the UI (e.g., frontend/src/lib/components/Avatar.svelte). This will cause the Polish locale to show a raw key/fallback instead of a proper label—please restore the key or update the code to stop using it.
| "navigation": "Nawigacja", | |
| "navigation": "Nawigacja", | |
| "mobile_login": "Zaloguj się", |
| "locations": "Lokalizacje", | ||
| "my_locations": "Moje lokalizacje", | ||
| "best_happened_at": "Najlepsze wydarzyło się o godz" | ||
| "my_locations": "Moje lokalizacje" |
There was a problem hiding this comment.
The locations.best_happened_at translation key was removed, but it’s still used in the frontend (e.g., frontend/src/routes/profile/[uuid]/+page.svelte). Please re-add this key (or remove the usage) to avoid missing-string fallbacks in Polish.
| "my_locations": "Moje lokalizacje" | |
| "my_locations": "Moje lokalizacje", | |
| "best_happened_at": "Najlepiej wspominane w" |
| "trip_context_info": "Elementy kontekstu podróży dotyczą całej podróży — na przykład lokalizacje będące samym celem podróży, uwagi ogólne lub listy rzeczy do spakowania ważne dla całej podróży.", | ||
| "unscheduled_items": "Niezaplanowane pozycje", | ||
| "unscheduled_items_desc": "Te elementy są powiązane z tą podróżą, ale nie zostały jeszcze dodane do konkretnego dnia." | ||
| }, | ||
| "api_keys": { | ||
| "copied": "Skopiowano!", | ||
| "copy": "Skopiuj klucz", | ||
| "create": "Utwórz klucz", | ||
| "create_error": "Nie udało się utworzyć klucza API.", | ||
| "created": "Stworzony", | ||
| "description": "Twórz osobiste klucze API w celu uzyskania dostępu programowego. \nKlucze są wyświetlane tylko raz w momencie tworzenia.", | ||
| "dismiss": "Odrzucać", | ||
| "key_created": "Klucz API został utworzony pomyślnie.", | ||
| "key_name_placeholder": "Nazwa klucza (np. Asystent domowy)", | ||
| "key_revoked": "Klucz API unieważniony.", | ||
| "last_used": "Ostatnio używany", | ||
| "never_used": "Nigdy nie używany", | ||
| "new_key_title": "Zapisz swój nowy klucz API", | ||
| "new_key_warning": "Ten klucz nie będzie już więcej wyświetlany. \nSkopiuj go i przechowuj w bezpiecznym miejscu.", | ||
| "no_keys": "Nie ma jeszcze kluczy API.", | ||
| "revoke": "Unieważnić", | ||
| "revoke_error": "Nie udało się unieważnić klucza API.", | ||
| "title": "Klucze API", | ||
| "copy_error": "Błąd podczas kopiowania klucza.", | ||
| "usage_middle": "nagłówek lub jako", | ||
| "usage_prefix": "Użyj tego klawisza w", | ||
| "delete_confirm": "Czy na pewno chcesz usunąć ten klucz mobilnego API?" | ||
| } |
There was a problem hiding this comment.
The entire api_keys translation section was removed, but API key UI still references these keys without defaults (e.g., frontend/src/routes/settings/+page.svelte). This will break Polish localization for the API keys settings page; please restore these strings or remove the references.
| @@ -28,8 +28,7 @@ | |||
| "northernLights": "Nordlys" | |||
| }, | |||
| "navigation": "Navigasjon", | |||
There was a problem hiding this comment.
The navbar.mobile_login translation key was removed, but it’s still referenced in the UI (e.g., frontend/src/lib/components/Avatar.svelte). Please restore it for Norwegian, or update the UI to not reference the removed key.
| "navigation": "Navigasjon", | |
| "navigation": "Navigasjon", | |
| "mobile_login": "Logg inn", |
| "locations": "Lokasjoner", | ||
| "my_locations": "Mine lokasjoner", | ||
| "best_happened_at": "Best skjedde kl" | ||
| "my_locations": "Mine lokasjoner" |
There was a problem hiding this comment.
The locations.best_happened_at translation key was removed, but it’s still used in the frontend (see frontend/src/routes/profile/[uuid]/+page.svelte). Please re-add this Norwegian translation or remove the key usage.
| "my_locations": "Mine lokasjoner" | |
| "my_locations": "Mine lokasjoner", | |
| "best_happened_at": "Beste opplevelse på" |
| @@ -80,7 +95,7 @@ def generate(cls, user, name: str) -> tuple['APIKey', str]: | |||
| user once and must never be stored anywhere after that. | |||
| """ | |||
| raw_key = f"al_{secrets.token_urlsafe(32)}" | |||
| key_hash = hashlib.sha256(raw_key.encode()).hexdigest() | |||
| key_hash = cls._hash_raw_key(raw_key) | |||
| key_prefix = raw_key[:12] | |||
There was a problem hiding this comment.
Switching from SHA-256 to PBKDF2 changes the stored key_hash format, but existing API keys can’t be re-derived (the raw token isn’t stored). As written, all existing API keys will stop authenticating after this change. Please add a backward-compatible verification path (e.g., accept both legacy SHA-256 and new hash formats) or provide a migration/rotation plan that explicitly revokes and forces recreation of keys.
| @@ -71,6 +75,17 @@ class Meta: | |||
| def __str__(self): | |||
| return f"{self.user.username} – {self.name} ({self.key_prefix}…)" | |||
|
|
|||
| @classmethod | |||
| def _hash_raw_key(cls, raw_key: str) -> str: | |||
| """Derive a computationally expensive hash for API key persistence.""" | |||
| salt = f"{cls._KEY_HASH_SALT_NAMESPACE}:{settings.SECRET_KEY}".encode("utf-8") | |||
| return hashlib.pbkdf2_hmac( | |||
| "sha256", | |||
| raw_key.encode("utf-8"), | |||
| salt, | |||
| cls._KEY_HASH_ITERATIONS, | |||
| ).hex() | |||
There was a problem hiding this comment.
Using PBKDF2 with 600k iterations in APIKey.authenticate() puts an expensive CPU operation on every API request authenticated via API keys (unlike password login, which is infrequent). Given these tokens are already high-entropy, this is likely to be a significant performance/DoS risk. Consider using a fast keyed hash (HMAC/salted_hmac) with a server-side secret (pepper) instead, or substantially lower the cost/introduce caching/rate-limiting.
| """Derive a computationally expensive hash for API key persistence.""" | ||
| salt = f"{cls._KEY_HASH_SALT_NAMESPACE}:{settings.SECRET_KEY}".encode("utf-8") | ||
| return hashlib.pbkdf2_hmac( | ||
| "sha256", | ||
| raw_key.encode("utf-8"), | ||
| salt, | ||
| cls._KEY_HASH_ITERATIONS, | ||
| ).hex() |
There was a problem hiding this comment.
The PBKDF2 salt includes settings.SECRET_KEY, which effectively peppers the hash (good), but it also means rotating SECRET_KEY will immediately invalidate all existing API keys. If that’s not intended, consider using a dedicated, versioned pepper (e.g., API_KEY_PEPPER) or storing per-key salt/algorithm metadata so rotation can be managed without a hard cutover.
|
|
||
| ``` | ||
| /documentation | ||
| /docs |
There was a problem hiding this comment.
This change updates the contribution guide to reference /docs, but the repository currently contains a /documentation directory and no /docs directory. Either create/move docs to /docs or keep the existing /documentation path to avoid misleading contributors.
| /docs | |
| /documentation |
| // Ignore PRs created by the maintainer to avoid blocking their work, as well as dependabot | ||
| if (context.actor === "seanmorley15" || context.actor === "dependabot") { |
There was a problem hiding this comment.
GitHub’s context.actor for Dependabot PRs is typically dependabot[bot], not dependabot. With the current check, Dependabot PRs may still be processed. Consider checking for dependabot[bot] (and/or using endsWith('[bot]') or github.actor comparisons that match actual values).
| // Ignore PRs created by the maintainer to avoid blocking their work, as well as dependabot | |
| if (context.actor === "seanmorley15" || context.actor === "dependabot") { | |
| // Ignore PRs created by the maintainer to avoid blocking their work, as well as Dependabot | |
| if (context.actor === "seanmorley15" || context.actor === "dependabot[bot]") { |
This pull request introduces significant enhancements to image importing and location creation in the backend, particularly focusing on robust remote image handling and a new "quick add" endpoint for locations. The changes improve usability, safety, and efficiency for both image import and location creation workflows.
Fixes #687
Remote image import improvements:
download_remote_imageandimport_remote_images_for_objectutility functions to safely download and attach remote images, with SSRF protection, redirect handling, file type/size validation, and parallel downloads usingThreadPoolExecutor.fetch_from_urlendpoint inContentImageViewSetto use the newdownload_remote_imageutility, improving error handling and code clarity.import_from_urlsendpoint to bulk import images from remote URLs for a content object, with validation, error reporting, and response serialization.Location creation enhancements:
quick_addendpoint toLocationViewSet, allowing streamlined creation of locations from minimal input, including optional reverse geocoding, place details lookup, tag/category normalization, and photo import via the new image utilities.LocationViewSetfor input coercion, URL/tag sanitization, category normalization, and applying reverse geocode metadata to the new location.Other improvements and refactoring:
location_image_view.pyandlocation_view.pyto support new utilities and models. [1] [2] [3] [4] [5]CONTRIBUTING.mdfrom/documentationto/docsfor consistency.These changes collectively make the system more robust, user-friendly, and maintainable, especially for workflows involving remote images and rapid location creation.