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

Add Shop context to logs #1850

Merged
merged 4 commits into from
Jan 7, 2025
Merged
Changes from 1 commit
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
Next Next commit
Revert "Revert "Add Shop context to logs""
  • Loading branch information
lizkenyon authored Dec 9, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
commit 5cc1d154b820164117a4ae7623ab6a2367be31b0
Original file line number Diff line number Diff line change
@@ -9,6 +9,7 @@ import {
respondToBotRequest,
respondToOptionsRequest,
validateSessionToken,
getShopFromRequest,
} from '../helpers';

import {
@@ -60,7 +61,9 @@ export function authStrategyFactory<
const url = new URL(request.url);

if (url.pathname === config.auth.patchSessionTokenPath) {
logger.debug('Rendering bounce page');
logger.debug('Rendering bounce page', {
shop: getShopFromRequest(request),
});
throw renderAppBridge({config, logger, api}, request);
}
}
@@ -71,7 +74,10 @@ export function authStrategyFactory<
if (url.pathname === config.auth.exitIframePath) {
const destination = url.searchParams.get('exitIframe')!;

logger.debug('Rendering exit iframe page', {destination});
logger.debug('Rendering exit iframe page', {
shop: getShopFromRequest(request),
destination,
});
throw renderAppBridge({config, logger, api}, request, {url: destination});
}
}
@@ -154,12 +160,14 @@ export function authStrategyFactory<
await ensureSessionTokenSearchParamIfRequired(params, request);
}

logger.info('Authenticating admin request');
logger.info('Authenticating admin request', {
shop: getShopFromRequest(request),
});

const {payload, shop, sessionId, sessionToken} =
await getSessionTokenContext(params, request);

logger.debug('Loading session from storage', {sessionId});
logger.debug('Loading session from storage', {shop, sessionId});
const existingSession = sessionId
? await config.sessionStorage!.loadSession(sessionId)
: undefined;
@@ -173,7 +181,9 @@ export function authStrategyFactory<
return createContext(request, session, strategy, payload);
} catch (errorOrResponse) {
if (errorOrResponse instanceof Response) {
logger.debug('Authenticate returned a response');
logger.debug('Authenticate returned a response', {
shop: getShopFromRequest(request),
});
ensureCORSHeadersFactory(params, request)(errorOrResponse);
}

@@ -193,6 +203,7 @@ async function getSessionTokenContext(
const sessionToken = (headerSessionToken || searchParamSessionToken)!;

logger.debug('Attempting to authenticate session token', {
shop: getShopFromRequest(request),
sessionToken: JSON.stringify({
header: headerSessionToken,
search: searchParamSessionToken,
@@ -204,7 +215,7 @@ async function getSessionTokenContext(
const dest = new URL(payload.dest);
const shop = dest.hostname;

logger.debug('Session token is valid', {shop, payload});
logger.debug('Session token is valid - authenticated', {shop, payload});
const sessionId = config.useOnlineTokens
? api.session.getJwtSessionId(shop, payload.sub)
: api.session.getOfflineId(shop);
Original file line number Diff line number Diff line change
@@ -11,7 +11,7 @@ export function redirectOutOfApp(
): never {
const {config, logger} = params;

logger.debug('Redirecting out of app', {url});
logger.debug('Redirecting out of app', {shop, url});

const requestUrl = new URL(request.url);
const isEmbeddedRequest = requestUrl.searchParams.get('embedded') === '1';
Original file line number Diff line number Diff line change
@@ -15,13 +15,15 @@ export function handleClientErrorFactory({
if (error instanceof HttpResponseError !== true) {
params.logger.debug(
`Got a response error from the API: ${error.message}`,
{shop: session.shop},
);
throw error;
}

params.logger.debug(
`Got an HTTP response error from the API: ${error.message}`,
{
shop: session.shop,
code: error.response.code,
statusText: error.response.statusText,
body: JSON.stringify(error.response.body),
Original file line number Diff line number Diff line change
@@ -46,7 +46,7 @@ export function redirectFactory(
init,
});

logger.debug('Redirecting', {url: parsedUrl.toString()});
logger.debug('Redirecting', {shop, url: parsedUrl.toString()});

const isSameOrigin = parsedUrl.origin === config.appUrl;
if (isSameOrigin || url.startsWith('/')) {
Original file line number Diff line number Diff line change
@@ -18,7 +18,7 @@ export async function triggerAfterAuthHook<
) {
const {config, logger} = params;
if (config.hooks.afterAuth) {
logger.info('Running afterAuth hook');
logger.info('Running afterAuth hook', {shop: session.shop});

const admin = createAdminApiContext<ConfigArg, Resources>(
session,
Original file line number Diff line number Diff line change
@@ -21,6 +21,7 @@ export function validateShopAndHostParams(
const host = api.utils.sanitizeHost(url.searchParams.get('host')!);
if (!host) {
logger.debug('Invalid host, redirecting to login path', {
shop,
host: url.searchParams.get('host'),
});
throw redirectToLoginPath(request, params);
Original file line number Diff line number Diff line change
@@ -190,7 +190,7 @@ export class AuthCodeFlowStrategy<
): Promise<never> {
const {api, config, logger} = this;

logger.info('Handling OAuth callback request');
logger.info('Handling OAuth callback request', {shop});

try {
const {session, headers: responseHeaders} = await api.auth.callback({
@@ -200,7 +200,9 @@ export class AuthCodeFlowStrategy<
await config.sessionStorage!.storeSession(session);

if (config.useOnlineTokens && !session.isOnline) {
logger.info('Requesting online access token for offline session');
logger.info('Requesting online access token for offline session', {
shop,
});
await beginAuth({api, config, logger}, request, true, shop);
}

@@ -273,7 +275,7 @@ export class AuthCodeFlowStrategy<
shop: string,
) {
const {logger} = this;
logger.error('Error during OAuth callback', {error: error.message});
logger.error('Error during OAuth callback', {shop, error: error.message});

if (error instanceof CookieNotFound) {
return this.handleAuthBeginRequest(request, shop);
Original file line number Diff line number Diff line change
@@ -10,6 +10,7 @@ import {BasicParams} from '../../../types';
import {ApiConfigWithFutureFlags, ApiFutureFlags} from '../../../future/flags';
import {HandleAdminClientError} from '../../../clients';
import {handleClientErrorFactory} from '../helpers';
import {getShopFromRequest} from '../../helpers';

import {AuthorizationStrategy, OnErrorOptions, SessionContext} from './types';

@@ -31,8 +32,10 @@ export class MerchantCustomAuth<Config extends AppConfigArg>
this.logger = logger;
}

public async respondToOAuthRequests(_request: Request): Promise<void> {
this.logger.debug('Skipping OAuth request for merchant custom app');
public async respondToOAuthRequests(request: Request): Promise<void> {
this.logger.debug('Skipping OAuth request for merchant custom app', {
shop: getShopFromRequest(request),
});
}

public async authenticate(
@@ -43,6 +46,7 @@ export class MerchantCustomAuth<Config extends AppConfigArg>

this.logger.debug(
'Building session from configured access token for merchant custom app',
{shop},
);
const session = this.api.session.customAppSession(shop);

Original file line number Diff line number Diff line change
@@ -12,6 +12,7 @@ import {BasicParams} from '../../../types';
import {
respondToInvalidSessionToken,
invalidateAccessToken,
getShopFromRequest,
} from '../../helpers';
import {handleClientErrorFactory, triggerAfterAuthHook} from '../helpers';
import {HandleAdminClientError} from '../../../clients';
@@ -52,8 +53,8 @@ export class TokenExchangeStrategy<Config extends AppConfigArg>
if (!sessionToken) throw new InvalidJwtError();

if (!session || !session.isActive(undefined)) {
logger.info('No valid session found');
logger.info('Requesting offline access token');
logger.info('No valid session found', {shop});
logger.info('Requesting offline access token', {shop});
const {session: offlineSession} = await this.exchangeToken({
request,
sessionToken,
@@ -66,7 +67,7 @@ export class TokenExchangeStrategy<Config extends AppConfigArg>
let newSession = offlineSession;

if (config.useOnlineTokens) {
logger.info('Requesting online access token');
logger.info('Requesting online access token', {shop});
const {session: onlineSession} = await this.exchangeToken({
request,
sessionToken,
@@ -113,7 +114,9 @@ export class TokenExchangeStrategy<Config extends AppConfigArg>
request,
onError: async ({session, error}: OnErrorOptions) => {
if (error.response.code === 401) {
logger.debug('Responding to invalid access token');
logger.debug('Responding to invalid access token', {
shop: getShopFromRequest(request),
});
await invalidateAccessToken({config, api, logger}, session);

respondToInvalidSessionToken({
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import {APP_URL, TEST_SHOP} from '../../../__test-helpers';
import {getShopFromRequest} from '../get-shop-from-request';

describe('getShopFromRequest', () => {
it('returns sanitized shop domain from request URL params', () => {
// GIVEN
const request = new Request(`${APP_URL}?shop=${TEST_SHOP}`);

// WHEN
const result = getShopFromRequest(request);

// THEN
expect(result).toBe(TEST_SHOP);
});
});
Original file line number Diff line number Diff line change
@@ -5,10 +5,12 @@ export async function createOrLoadOfflineSession(
{api, config, logger}: BasicParams,
) {
if (config.distribution === AppDistribution.ShopifyAdmin) {
logger.debug('Creating custom app session from configured access token');
logger.debug('Creating custom app session from configured access token', {
shop,
});
return api.session.customAppSession(shop);
} else {
logger.debug('Loading offline session from session storage');
logger.debug('Loading offline session from session storage', {shop});
const offlineSessionId = api.session.getOfflineId(shop);
const session = await config.sessionStorage!.loadSession(offlineSessionId);

Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export function getShopFromRequest(request: Request) {
const url = new URL(request.url);
return url.searchParams.get('shop')!;
}
Original file line number Diff line number Diff line change
@@ -8,3 +8,4 @@ export * from './reject-bot-request';
export * from './respond-to-options-request';
export * from './respond-to-invalid-session-token';
export * from './create-or-load-offline-session';
export * from './get-shop-from-request';
Original file line number Diff line number Diff line change
@@ -8,7 +8,9 @@ export async function invalidateAccessToken(
): Promise<void> {
const {logger, config} = params;

logger.debug(`Invalidating access token for session - ${session.id}`);
logger.debug(`Invalidating access token for session - ${session.id}`, {
shop: session.shop,
});

session.accessToken = undefined;
await config.sessionStorage!.storeSession(session);
Original file line number Diff line number Diff line change
@@ -3,6 +3,7 @@ import {JwtPayload} from '@shopify/shopify-api';
import type {BasicParams} from '../../types';

import {respondToInvalidSessionToken} from './respond-to-invalid-session-token';
import {getShopFromRequest} from './get-shop-from-request';

interface ValidateSessionTokenOptions {
checkAudience?: boolean;
@@ -15,19 +16,24 @@ export async function validateSessionToken(
{checkAudience = true}: ValidateSessionTokenOptions = {},
): Promise<JwtPayload> {
const {api, logger} = params;
logger.debug('Validating session token');
logger.debug('Validating session token', {shop: getShopFromRequest(request)});

try {
const payload = await api.session.decodeSessionToken(token, {
checkAudience,
});
logger.debug('Session token is valid', {
const dest = new URL(payload.dest);
const shop = dest.hostname;
logger.debug('Session token is valid - validated', {
shop,
payload: JSON.stringify(payload),
});

return payload;
} catch (error) {
logger.debug(`Failed to validate session token: ${error.message}`);
logger.debug(`Failed to validate session token: ${error.message}`, {
shop: getShopFromRequest(request),
});

throw respondToInvalidSessionToken({params, request, retryRequest: true});
}
Original file line number Diff line number Diff line change
@@ -22,19 +22,18 @@ export function authenticateAppProxyFactory<
): Promise<
AppProxyContext | AppProxyContextWithSession<ConfigArg, Resources>
> {
logger.info('Authenticating app proxy request');

const url = new URL(request.url);
const shop = url.searchParams.get('shop')!;
logger.info('Authenticating app proxy request', {shop});

if (!(await validateAppProxyHmac(params, url))) {
logger.info('App proxy request has invalid signature');
logger.info('App proxy request has invalid signature', {shop});
throw new Response(undefined, {
status: 400,
statusText: 'Bad Request',
});
}

const shop = url.searchParams.get('shop')!;
const sessionId = api.session.getOfflineId(shop);
const session = await config.sessionStorage!.loadSession(sessionId);

@@ -137,7 +136,8 @@ async function validateAppProxyHmac(

return isValid;
} catch (error) {
logger.info(error.message);
const shop = url.searchParams.get('shop')!;
logger.info(error.message, {shop});
throw new Response(undefined, {status: 400, statusText: 'Bad Request'});
}
}
Original file line number Diff line number Diff line change
@@ -5,6 +5,7 @@ import {
getSessionTokenHeader,
validateSessionToken,
ensureCORSHeadersFactory,
getShopFromRequest,
} from '../../helpers';

import {AuthenticateExtension, ExtensionContext} from './types';
@@ -26,10 +27,14 @@ export function authenticateExtensionFactory(

const sessionTokenHeader = getSessionTokenHeader(request);

logger.info(`Authenticating ${requestType} request`);
logger.info(`Authenticating ${requestType} request`, {
shop: getShopFromRequest(request),
});

if (!sessionTokenHeader) {
logger.debug('Request did not contain a session token');
logger.debug('Request did not contain a session token', {
shop: getShopFromRequest(request),
});
throw new Response(undefined, {
status: 401,
statusText: 'Unauthorized',