Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

map-specific access token #8364

Merged
merged 19 commits into from
Jul 10, 2019
Merged
Show file tree
Hide file tree
Changes from 9 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
47 changes: 47 additions & 0 deletions debug/token.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<!DOCTYPE html>
<html>
<head>
<title>Mapbox GL JS debug page</title>
<meta charset='utf-8'>
<meta name="viewport" content="width=device-width, initial-scale=1.0, user-scalable=no">
<link rel='stylesheet' href='/dist/mapbox-gl.css' />
<style>
body { margin: 0; padding: 0; }
html, body, #map { height: 100%; }
</style>
</head>

<body>
<div id='map'></div>
<div id='hidden' style='display:none'></div>
<script src='/dist/mapbox-gl-dev.js'></script>
<script src='/debug/access_token_generated.js'></script>
peterqliu marked this conversation as resolved.
Show resolved Hide resolved
<script>

// 1) first map should work with its valid local accessToken, despite the bad global mapboxgl token
// 2) first map should keep working properly, despite the bad token in the second map

var mapToken = mapboxgl.accessToken;
mapboxgl.accessToken = 'notAToken';

var map = window.map = new mapboxgl.Map({
container: 'map',
zoom: 12.5,
center: [-77.01866, 38.888],
style: 'mapbox://styles/mapbox/streets-v10',
hash: true,
accessToken: mapToken
});

var map2 = new mapboxgl.Map({
container: 'hidden',
zoom: 12.5,
center: [-77.01866, 38.888],
style: 'mapbox://styles/mapbox/dark-v10',
hash: true,
accessToken: 'anotherBadToken'
});

</script>
</body>
</html>
1 change: 1 addition & 0 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ const exported = {
},

set baseApiUrl(url: string) {
console.log('setting');
Copy link
Contributor

Choose a reason for hiding this comment

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

console.log

config.API_URL = url;
},

Expand Down
4 changes: 2 additions & 2 deletions src/source/vector_tile_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ class VectorTileSource extends Evented implements Source {
extend(this, tileJSON);
if (tileJSON.bounds) this.tileBounds = new TileBounds(tileJSON.bounds, this.minzoom, this.maxzoom);

postTurnstileEvent(tileJSON.tiles);
postMapLoadEvent(tileJSON.tiles, this.map._getMapId(), this.map._requestManager._skuToken);
postTurnstileEvent(tileJSON.tiles, this.map._requestManager._customAccessToken);
postMapLoadEvent(tileJSON.tiles, this.map._getMapId(), this.map._requestManager._skuToken, this.map._requestManager._customAccessToken);

// `content` is included here to prevent a race condition where `Style#_updateSources` is called
// before the TileJSON arrives. this makes sure the tiles needed are loaded once TileJSON arrives
Expand Down
1 change: 0 additions & 1 deletion src/style/style.js
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,6 @@ class Style extends Evented {

url = this.map._requestManager.normalizeStyleURL(url, options.accessToken);
const request = this.map._requestManager.transformRequest(url, ResourceType.Style);

this._request = getJSON(request, (error: ?Error, json: ?Object) => {
this._request = null;
if (error) {
Expand Down
8 changes: 6 additions & 2 deletions src/ui/map.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@ type MapOptions = {
pitch?: number,
renderWorldCopies?: boolean,
maxTileCacheSize?: number,
transformRequest?: RequestTransformFunction
transformRequest?: RequestTransformFunction,
accessToken: string
};

const defaultMinZoom = 0;
Expand Down Expand Up @@ -211,6 +212,8 @@ const defaultOptions = {
* @param {boolean} [options.collectResourceTiming=false] If `true`, Resource Timing API information will be collected for requests made by GeoJSON and Vector Tile web workers (this information is normally inaccessible from the main Javascript thread). Information will be returned in a `resourceTiming` property of relevant `data` events.
* @param {number} [options.fadeDuration=300] Controls the duration of the fade-in/fade-out animation for label collisions, in milliseconds. This setting affects all symbol layers. This setting does not affect the duration of runtime styling transitions or raster tile cross-fading.
* @param {boolean} [options.crossSourceCollisions=true] If `true`, symbols from multiple sources can collide with each other during collision detection. If `false`, collision detection is run separately for the symbols in each source.
* @param {string} [options.accessToken=null] If specified, map will use this token instead of the one defined in mapboxgl.accessToken.

* @example
* var map = new mapboxgl.Map({
* container: 'map',
Expand Down Expand Up @@ -331,7 +334,8 @@ class Map extends Camera {
this._renderTaskQueue = new TaskQueue();
this._controls = [];
this._mapId = uniqueId();
this._requestManager = new RequestManager(options.transformRequest);

this._requestManager = new RequestManager(options.transformRequest, options.accessToken);

if (typeof options.container === 'string') {
this._container = window.document.getElementById(options.container);
Expand Down
56 changes: 29 additions & 27 deletions src/util/mapbox.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,9 @@ export class RequestManager {
_skuTokenExpiresAt: number;
_transformRequestFn: ?RequestTransformFunction;

constructor(transformRequestFn?: RequestTransformFunction) {
constructor(transformRequestFn?: RequestTransformFunction, customAccessToken?: string) {
this._transformRequestFn = transformRequestFn;
this._customAccessToken = customAccessToken;
this._createSkuToken();
}

Expand All @@ -66,27 +67,27 @@ export class RequestManager {
}

normalizeStyleURL(url: string, accessToken?: string): string {
return normalizeStyleURL(url, accessToken);
return normalizeStyleURL(url, this._customAccessToken || accessToken);
}

normalizeGlyphsURL(url: string, accessToken?: string): string {
return normalizeGlyphsURL(url, accessToken);
return normalizeGlyphsURL(url, this._customAccessToken || accessToken);
}

normalizeSourceURL(url: string, accessToken?: string): string {
return normalizeSourceURL(url, accessToken);
return normalizeSourceURL(url, this._customAccessToken || accessToken);
}

normalizeSpriteURL(url: string, format: string, extension: string, accessToken?: string): string {
return normalizeSpriteURL(url, format, extension, accessToken);
return normalizeSpriteURL(url, format, extension, this._customAccessToken || accessToken);
}

normalizeTileURL(tileURL: string, sourceURL?: ?string, tileSize?: ?number): string {
if (this._isSkuTokenExpired()) {
this._createSkuToken();
}

return normalizeTileURL(tileURL, sourceURL, tileSize, this._skuToken);
return normalizeTileURL(tileURL, sourceURL, tileSize, this._skuToken, this._customAccessToken);
}

canonicalizeTileURL(url: string) {
Expand Down Expand Up @@ -166,7 +167,7 @@ const normalizeSpriteURL = function(url: string, format: string, extension: stri

const imageExtensionRe = /(\.(png|jpg)\d*)(?=$)/;

const normalizeTileURL = function(tileURL: string, sourceURL?: ?string, tileSize?: ?number, skuToken?: string): string {
const normalizeTileURL = function(tileURL: string, sourceURL?: ?string, tileSize?: ?number, skuToken?: string, customAccessToken?: string): string {
if (!sourceURL || !isMapboxURL(sourceURL)) return tileURL;

const urlObject = parseUrl(tileURL);
Expand All @@ -179,11 +180,11 @@ const normalizeTileURL = function(tileURL: string, sourceURL?: ?string, tileSize
urlObject.path = urlObject.path.replace(imageExtensionRe, `${suffix}${extension}`);
urlObject.path = `/v4${urlObject.path}`;

if (config.REQUIRE_ACCESS_TOKEN && config.ACCESS_TOKEN && skuToken) {
if (config.REQUIRE_ACCESS_TOKEN && RequestManager._customAccessToken || config.ACCESS_TOKEN && skuToken) {
urlObject.params.push(`sku=${skuToken}`);
}

return makeAPIURL(urlObject);
return makeAPIURL(urlObject, customAccessToken);
peterqliu marked this conversation as resolved.
Show resolved Hide resolved
};

// matches any file extension specified by a dot and one or more alphanumeric characters
Expand Down Expand Up @@ -279,12 +280,12 @@ class TelemetryEvent {
}

getStorageKey(domain: ?string) {
const tokenData = parseAccessToken(config.ACCESS_TOKEN);
const tokenData = parseAccessToken(RequestManager._customAccessToken || config.ACCESS_TOKEN);
let u = '';
if (tokenData && tokenData['u']) {
u = b64EncodeUnicode(tokenData['u']);
} else {
u = config.ACCESS_TOKEN || '';
u = RequestManager._customAccessToken || config.ACCESS_TOKEN || '';
}
return domain ?
`${telemEventKey}.${domain}:${u}` :
Expand Down Expand Up @@ -336,10 +337,11 @@ class TelemetryEvent {
* to the values that should be saved. For this reason, the callback should be invoked prior to the call
* to TelemetryEvent#saveData
*/
postEvent(timestamp: number, additionalPayload: {[string]: any}, callback: (err: ?Error) => void) {
postEvent(timestamp: number, additionalPayload: {[string]: any}, callback: (err: ?Error) => void, customAccessToken: string) {
if (!config.EVENTS_URL) return;
const eventsUrlObject: UrlObject = parseUrl(config.EVENTS_URL);
eventsUrlObject.params.push(`access_token=${config.ACCESS_TOKEN || ''}`);
eventsUrlObject.params.push(`access_token=${customAccessToken || config.ACCESS_TOKEN || ''}`);

const payload: Object = {
event: this.type,
created: new Date(timestamp).toISOString(),
Expand All @@ -362,13 +364,13 @@ class TelemetryEvent {
this.pendingRequest = null;
callback(error);
this.saveEventData();
this.processRequests();
this.processRequests(customAccessToken);
});
}

queueRequest(event: number | {id: number, timestamp: number}) {
queueRequest(event: number | {id: number, timestamp: number}, customAccessToken: string) {
this.queue.push(event);
this.processRequests();
this.processRequests(customAccessToken);
}
}

Expand All @@ -382,20 +384,20 @@ export class MapLoadEvent extends TelemetryEvent {
this.skuToken = '';
}

postMapLoadEvent(tileUrls: Array<string>, mapId: number, skuToken: string) {
postMapLoadEvent(tileUrls: Array<string>, mapId: number, skuToken: string, customAccessToken: string) {
//Enabled only when Mapbox Access Token is set and a source uses
// mapbox tiles.
this.skuToken = skuToken;

if (config.EVENTS_URL &&
config.ACCESS_TOKEN &&
customAccessToken || config.ACCESS_TOKEN &&
Array.isArray(tileUrls) &&
tileUrls.some(url => isMapboxURL(url) || isMapboxHTTPURL(url))) {
this.queueRequest({id: mapId, timestamp: Date.now()});
this.queueRequest({id: mapId, timestamp: Date.now()}, customAccessToken);
}
}

processRequests() {
processRequests(customAccessToken) {
if (this.pendingRequest || this.queue.length === 0) return;
const {id, timestamp} = this.queue.shift();

Expand All @@ -414,7 +416,7 @@ export class MapLoadEvent extends TelemetryEvent {
if (!err) {
if (id) this.success[id] = true;
}
});
}, customAccessToken);
}
}

Expand All @@ -423,19 +425,19 @@ export class TurnstileEvent extends TelemetryEvent {
super('appUserTurnstile');
}

postTurnstileEvent(tileUrls: Array<string>) {
postTurnstileEvent(tileUrls: Array<string>, customAccessToken: string) {
//Enabled only when Mapbox Access Token is set and a source uses
// mapbox tiles.
if (config.EVENTS_URL &&
config.ACCESS_TOKEN &&
Array.isArray(tileUrls) &&
tileUrls.some(url => isMapboxURL(url) || isMapboxHTTPURL(url))) {
this.queueRequest(Date.now());
this.queueRequest(Date.now(), customAccessToken);
}
}


processRequests() {
processRequests(customAccessToken) {
if (this.pendingRequest || this.queue.length === 0) {
return;
}
Expand All @@ -445,8 +447,8 @@ export class TurnstileEvent extends TelemetryEvent {
this.fetchEventData();
}

const tokenData = parseAccessToken(config.ACCESS_TOKEN);
const tokenU = tokenData ? tokenData['u'] : config.ACCESS_TOKEN;
const tokenData = parseAccessToken(RequestManager._customAccessToken || config.ACCESS_TOKEN);
peterqliu marked this conversation as resolved.
Show resolved Hide resolved
const tokenU = tokenData ? tokenData['u'] : RequestManager._customAccessToken || config.ACCESS_TOKEN;
//Reset event data cache if the access token owner changed.
let dueForEvent = tokenU !== this.eventData.tokenU;

Expand Down Expand Up @@ -475,7 +477,7 @@ export class TurnstileEvent extends TelemetryEvent {
this.eventData.lastSuccess = nextUpdate;
this.eventData.tokenU = tokenU;
}
});
}, customAccessToken);
}
}

Expand Down
13 changes: 13 additions & 0 deletions test/unit/ui/map.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,19 @@ test('Map', (t) => {
t.end();
});

// t.test('map-specific token', (t) => {\
Copy link
Contributor Author

Choose a reason for hiding this comment

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

How can we instantiate a map with a bad token?

Would like to add a test for the positive case, where it starts with mapboxgl.accessToken = badtoken but gets overwritten by a valid token inside the map options.

Copy link
Contributor

Choose a reason for hiding this comment

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

How can we instantiate a map with a bad token?
The unit tests don't exercise any access token specific logic.

Map options are set-only with no ability to read back to options that were used to instantiate a map object. Since the access token is passed through to the RequestManager unit tests are probably suited to the behavior of that class.

// t.end();
// });

t.test('bad map-specific token breaks map', (t) => {
const container = window.document.createElement('div');
Object.defineProperty(container, 'offsetWidth', {value: 512});
Object.defineProperty(container, 'offsetHeight', {value: 512});
createMap(t, {accessToken:'notAToken'});
t.error();
t.end();
});

t.test('initial bounds in constructor options', (t) => {
const container = window.document.createElement('div');
Object.defineProperty(container, 'offsetWidth', {value: 512});
Expand Down