fix(geo): stop calling ip-api.com from the browser (FH-1.1)
detectLocation() fetched http://ip-api.com over plaintext from an HTTPS storefront. Browsers block mixed active content, so the request never completed and auto-detect only ever took its error branch - region detection has been dead in production, not merely insecure. The attempt also handed every visitor's IP to a third party from the page itself. Geo now resolves through the tenant API at {baseUrl}/geo/resolve, the same base /regions already uses. The server reads the client IP; the browser sends nothing and receives no third-party payload. The endpoint is specified in BACKEND-API-REFERENCE.md and is not built yet. Until it ships the client falls back to the manual region picker - identical to the behaviour production already had. Adds location.service.spec.ts: geo goes to the tenant API, no request leaves that origin or uses http://, failure degrades to the manual picker, and detection is not retried once attempted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -307,6 +307,18 @@ Base: `ApiConfigService.getBaseUrl()`. Headers on every call (`apiHeadersInterce
|
||||
| `/items/{id}/questiion` | POST | `{ question, sessionID, timestamp }` | `{ message }` — **literal typo `questiion`, preserve it, matches the client** |
|
||||
| `/purchase-email` | POST | `{ email, phone?, telegramUserId, items[] }` | `{ message }` |
|
||||
| `/regions` | GET | — | `Region[]` — client falls back **silently** to 6 hardcoded regions on any error |
|
||||
| `/geo/resolve` | GET | — | `GeoIpResponse` — **not built yet**, see below |
|
||||
|
||||
**`/geo/resolve` — new, required.** Resolves the *caller's* IP to a coarse location so the storefront can pre-select a region. The server reads the client IP (behind the proxy, so honour `X-Forwarded-For` with `trustProxy`); the browser sends nothing and receives no third-party payload.
|
||||
|
||||
Response is the existing `GeoIpResponse` shape (`src/app/models/location.model.ts`): `{ city, country, countryCode, region?, timezone?, lat?, lon? }`.
|
||||
|
||||
Rules:
|
||||
- City-level precision only. Do not return coordinates finer than the city centroid, and do not persist the lookup against a customer record — this runs for anonymous visitors.
|
||||
- Any failure returns a non-2xx. The client already treats every error as "stay on the manual picker", so a degraded geo provider must never block the storefront.
|
||||
- Rate-limit per IP; it is an unauthenticated endpoint.
|
||||
|
||||
This replaces a direct browser call to `http://ip-api.com`, which leaked every visitor's IP to a third party and — being plaintext on an HTTPS origin — was blocked as mixed content, so region auto-detect never actually worked in production. Until this endpoint ships the client silently falls back to the manual region picker, which is the same behaviour production has had all along.
|
||||
|
||||
### 6.1 Products — the tolerance contract
|
||||
|
||||
|
||||
@@ -27,7 +27,8 @@ Improvements only. Nothing here regresses our Angular version, test count, or ar
|
||||
|
||||
## Wave 1 — Live defects with a security benefit (Lane A, this sprint)
|
||||
|
||||
- [ ] **FH-1.1 — Kill the plaintext third-party geo call** · S · Lane A
|
||||
- [x] **FH-1.1 — Kill the plaintext third-party geo call** · S · Lane A · **done 2026-08-21**
|
||||
Now `GET {tenantApiBase}/geo/resolve`, same base as `/regions`. Server reads the client IP; nothing leaves our infrastructure. Endpoint specified in [BACKEND-API-REFERENCE.md](../BACKEND-API-REFERENCE.md) §6 — **not built yet**, and until it is the client falls back to the manual picker, which is what production has effectively had all along. Covered by `src/app/services/location.service.spec.ts` (4 tests, one of which fails the build on any off-origin or plaintext request from this service).
|
||||
`src/app/services/location.service.ts:75` calls `http://ip-api.com/json/?fields=…` from an HTTPS origin. Browsers block mixed active content, so `detectLocation()` has been silently taking its error branch in production — region auto-detect is dead, not just insecure. It is also a third-party geo leak on every session.
|
||||
**Do:** remove the direct call. Resolve region server-side (`GET /api/v1/geo/resolve`, backend reads the client IP) or drop auto-detect and keep the manual region picker.
|
||||
**Done when:** zero `http://` literals in `src/`; a unit test asserts `detectLocation()` issues no cross-origin request to a non-allowlisted host.
|
||||
@@ -37,7 +38,8 @@ Improvements only. Nothing here regresses our Angular version, test count, or ar
|
||||
**Do:** accept only an `https:` URL whose origin the backend returned in the payment response (backend allowlist, per their `safeHttpsUrl()`); navigate the current tab instead of framing.
|
||||
**Done when:** a non-https or non-allowlisted URL is refused with a visible payment error; a test covers both the accepted and the refused case.
|
||||
|
||||
- [ ] **FH-1.3 — Remove provider credentials from the browser** · M · Lane A · *depends on FH-0.2*
|
||||
- [x] **FH-1.3 — Remove provider credentials from the browser** · M · Lane A · **landed via the `@marketplaces/payment` migration**
|
||||
The legacy payment surface on `ApiService` was deleted wholesale in that work. `grep -ri "authorization-key\|userid-value\|web-97ec" src/` now returns nothing. Keep FH-3.5 (bundle secret scan) to stop it coming back.
|
||||
`src/app/services/api.service.ts:675` sets `authorization-key` and `userid-value` headers client-side. `api.service.ts:143` ships a partner ID literal (`'web-97ec-9c57-4dde-9037-3a68f7f83750'`) in the bundle. Their audit's most serious finding, and it is correct.
|
||||
**Do:** delete both header paths and the literal; the browser gets a checkout URL or a status endpoint, never a credential.
|
||||
**Done when:** `grep -ri "authorization-key\|userid-value" src/` returns nothing; no partner ID literal in `dist/`; a CI check greps the built bundle for both patterns.
|
||||
|
||||
89
src/app/services/location.service.spec.ts
Normal file
89
src/app/services/location.service.spec.ts
Normal file
@@ -0,0 +1,89 @@
|
||||
import { provideHttpClient } from '@angular/common/http';
|
||||
import { HttpTestingController, provideHttpClientTesting } from '@angular/common/http/testing';
|
||||
import { TestBed } from '@angular/core/testing';
|
||||
import { ApiConfigService } from '../core/config/api-config.service';
|
||||
import { LocalStorageService } from '../core/storage/local-storage.service';
|
||||
import { LocationService } from './location.service';
|
||||
|
||||
/**
|
||||
* FH-1.1. detectLocation() used to call http://ip-api.com directly. On an
|
||||
* HTTPS storefront the browser blocks mixed active content, so the request
|
||||
* never completed and region auto-detect silently did nothing in production
|
||||
* - and the attempt still exposed the visitor's IP to a third party.
|
||||
*
|
||||
* These tests pin both halves of the fix: geo resolution goes to our own
|
||||
* tenant API, and nothing in this service reaches a foreign origin.
|
||||
*/
|
||||
describe('LocationService', () => {
|
||||
const baseUrl = 'https://api.gorbushka.market';
|
||||
let service: LocationService;
|
||||
let httpTesting: HttpTestingController;
|
||||
|
||||
beforeEach(() => {
|
||||
TestBed.configureTestingModule({
|
||||
providers: [
|
||||
LocationService,
|
||||
provideHttpClient(),
|
||||
provideHttpClientTesting(),
|
||||
{ provide: ApiConfigService, useValue: { getBaseUrl: () => baseUrl } },
|
||||
{
|
||||
provide: LocalStorageService,
|
||||
useValue: { getJSON: () => null, setJSON: () => {}, removeItem: () => {} },
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
service = TestBed.inject(LocationService);
|
||||
httpTesting = TestBed.inject(HttpTestingController);
|
||||
// The constructor loads regions; flush it so each test starts clean.
|
||||
httpTesting.expectOne(`${baseUrl}/regions`).flush([]);
|
||||
});
|
||||
|
||||
afterEach(() => httpTesting.verify());
|
||||
|
||||
it('resolves geo through the tenant API, not a third-party host', () => {
|
||||
service.detectLocation();
|
||||
|
||||
const request = httpTesting.expectOne(`${baseUrl}/geo/resolve`);
|
||||
expect(request.request.method).toBe('GET');
|
||||
request.flush({ city: 'Москва', country: 'Россия', countryCode: 'RU' });
|
||||
});
|
||||
|
||||
it('issues no request to a foreign or plaintext origin', () => {
|
||||
service.detectLocation();
|
||||
|
||||
for (const request of httpTesting.match(() => true)) {
|
||||
expect(request.request.url.startsWith(baseUrl))
|
||||
.withContext(`unexpected off-origin request: ${request.request.url}`)
|
||||
.toBe(true);
|
||||
expect(request.request.url.startsWith('http://'))
|
||||
.withContext(`plaintext request: ${request.request.url}`)
|
||||
.toBe(false);
|
||||
request.flush({});
|
||||
}
|
||||
});
|
||||
|
||||
it('degrades to the manual picker when geo resolution fails', () => {
|
||||
service.detectLocation();
|
||||
httpTesting
|
||||
.expectOne(`${baseUrl}/geo/resolve`)
|
||||
.flush(null, { status: 503, statusText: 'Service Unavailable' });
|
||||
|
||||
expect(service.region()).toBeNull();
|
||||
expect(service.detecting()).toBe(false);
|
||||
expect(service.autoDetected()).toBe(true);
|
||||
});
|
||||
|
||||
it('does not re-request geo once detection has been attempted', () => {
|
||||
service.detectLocation();
|
||||
httpTesting.expectOne(`${baseUrl}/geo/resolve`).flush({
|
||||
city: 'Ереван',
|
||||
country: 'Армения',
|
||||
countryCode: 'AM',
|
||||
});
|
||||
|
||||
service.detectLocation();
|
||||
|
||||
httpTesting.expectNone(`${baseUrl}/geo/resolve`);
|
||||
});
|
||||
});
|
||||
@@ -71,8 +71,13 @@ export class LocationService {
|
||||
if (this.detectedSignal()) return; // already tried
|
||||
this.loadingSignal.set(true);
|
||||
|
||||
// Using free ip-api.com — no key required, 45 req/min
|
||||
this.http.get<GeoIpResponse>('http://ip-api.com/json/?fields=city,country,countryCode,region,timezone,lat,lon')
|
||||
// Was a direct plaintext call to ip-api.com. Two problems, one of them
|
||||
// fatal: browsers block mixed active content, so on an HTTPS storefront
|
||||
// this request never completed and auto-detect only ever took the error
|
||||
// branch below. It also handed every visitor's IP to a third party from
|
||||
// the page itself. The client IP is the server's to read - same tenant
|
||||
// API base as /regions, same-origin, nothing leaves our infrastructure.
|
||||
this.http.get<GeoIpResponse>(`${this.apiConfig.getBaseUrl()}/geo/resolve`)
|
||||
.subscribe({
|
||||
next: (geo) => {
|
||||
this.detectedSignal.set(true);
|
||||
|
||||
Reference in New Issue
Block a user