Skip to content

Commit

Permalink
#8791: Migrate /api/me/ from version 1.0 to 1.1 to remove telemetry o…
Browse files Browse the repository at this point in the history
…rganization (#9074)
  • Loading branch information
johnnymetz authored Sep 6, 2024
1 parent 65acc8e commit aec9226
Show file tree
Hide file tree
Showing 19 changed files with 3,671 additions and 864 deletions.
4 changes: 2 additions & 2 deletions .secrets.baseline
Original file line number Diff line number Diff line change
Expand Up @@ -262,14 +262,14 @@
"filename": "src/testUtils/factories/authFactories.ts",
"hashed_secret": "d15e5a27160ace913d22871497c05a7e5bbbe2ef",
"is_verified": false,
"line_number": 159
"line_number": 158
},
{
"type": "Hex High Entropy String",
"filename": "src/testUtils/factories/authFactories.ts",
"hashed_secret": "ff998abc1ce6d8f01a675fa197368e44c8916e9c",
"is_verified": false,
"line_number": 174
"line_number": 173
}
]
},
Expand Down
3 changes: 0 additions & 3 deletions src/auth/authSelectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,3 @@ export const selectOrganizations = (state: AuthRootState) =>
selectAuth(state).organizations;
export const selectOrganization = (state: AuthRootState) =>
selectAuth(state).organization;

export const selectTelemetryOrganizationId = (state: AuthRootState) =>
selectAuth(state).telemetryOrganizationId;
17 changes: 4 additions & 13 deletions src/auth/authTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,6 @@ export type UserData = Partial<{
* The user's primary organization.
*/
organizationId: UUID | null;
/**
* The user's organization for engagement and error attribution
*/
telemetryOrganizationId: UUID | null;
/**
* Organizations the user is a member of
*/
Expand Down Expand Up @@ -102,7 +98,6 @@ export type UserDataUpdate = Required<Except<UserData, "hostname" | "user">>;
export const USER_DATA_UPDATE_KEYS: Array<keyof UserDataUpdate> = [
"email",
"organizationId",
"telemetryOrganizationId",
"organizations",
"groups",
"enforceUpdateMillis",
Expand Down Expand Up @@ -157,6 +152,10 @@ export type OrganizationAuthState = {
* The human-readable name of the organization.
*/
readonly name: string;
/**
* Whether the organization is an enterprise organization.
*/
readonly isEnterprise: boolean;
/**
* The package scope of the organization, or null if not set.
*/
Expand Down Expand Up @@ -230,14 +229,6 @@ export type AuthState = {
*/
readonly organization?: OrganizationAuthState | null;

/**
* The enterprise organization used for telemetry collection, or null. Generally, if set, this will be the same as the
* user's primary organization.
*
* @since 1.7.35
*/
readonly telemetryOrganizationId?: UUID | null;

/**
* Organizations the user is a member of
*/
Expand Down
5 changes: 1 addition & 4 deletions src/auth/authUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ export async function getUserScope(): Promise<Nullishable<string>> {
export function selectUserDataUpdate({
email,
primaryOrganization,
telemetryOrganization,
organizationMemberships,
groupMemberships,
partner,
Expand All @@ -64,7 +63,6 @@ export function selectUserDataUpdate({
-- email is always present, pending above type refactoring */
email: email!,
organizationId: primaryOrganization?.organizationId ?? null,
telemetryOrganizationId: telemetryOrganization?.organizationId ?? null,
organizations,
groups,
partner: partner ?? null,
Expand All @@ -78,7 +76,6 @@ export function selectExtensionAuthState({
email,
scope,
primaryOrganization,
telemetryOrganization,
isOnboarded,
isTestAccount,
userMilestones: milestones,
Expand All @@ -98,6 +95,7 @@ export function selectExtensionAuthState({
: {
id: primaryOrganization.organizationId,
name: primaryOrganization.organizationName,
isEnterprise: primaryOrganization.isEnterprise,
scope: primaryOrganization.scope,
theme: primaryOrganization.organizationTheme,
control_room: primaryOrganization.controlRoom,
Expand All @@ -112,7 +110,6 @@ export function selectExtensionAuthState({
isTestAccount,
extension: true,
organization,
telemetryOrganizationId: telemetryOrganization?.organizationId,
organizations,
groups,
milestones,
Expand Down
5 changes: 0 additions & 5 deletions src/background/deploymentUpdater.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ import { checkDeploymentPermissions } from "@/permissions/deploymentPermissionsH
import { Events } from "@/telemetry/events";
import { allSettled } from "@/utils/promiseUtils";
import type { Manifest } from "webextension-polyfill";
import { getRequestHeadersByAPIVersion } from "@/data/service/apiVersioning";
import { fetchDeploymentModDefinitions } from "@/modDefinitions/modDefinitionRawApiCalls";
import { integrationConfigLocator } from "@/background/messenger/api";
import type { ActivatableDeployment } from "@/types/deploymentTypes";
Expand Down Expand Up @@ -566,10 +565,6 @@ export async function syncDeployments(): Promise<void> {
active: selectInstalledDeployments(activatedModComponents),
campaignIds,
},
{
// @since 1.8.10 -- API version 1.1 excludes the package config
headers: getRequestHeadersByAPIVersion("1.1"),
},
);

const isInitialDeploymentUpdate =
Expand Down
1 change: 1 addition & 0 deletions src/background/messenger/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ export const clearIntegrationRegistry = getMethod(
bg,
);
export const getUserData = getMethod("GET_USER_DATA", bg);
export const getMe = getMethod("GET_ME", bg);
export const activateWelcomeModsInBackground = getMethod(
"ACTIVATE_WELCOME_MODS",
bg,
Expand Down
3 changes: 3 additions & 0 deletions src/background/messenger/registration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ import { debouncedActivateWelcomeMods as activateWelcomeMods } from "@/backgroun
import { launchAuthIntegration } from "@/background/auth/partnerIntegrations/launchAuthIntegration";
import { getPartnerPrincipals } from "@/background/auth/partnerIntegrations/getPartnerPrincipals";
import refreshPartnerAuthentication from "@/background/auth/partnerIntegrations/refreshPartnerAuthentication";
import { getMe } from "@/data/service/backgroundApi";

expectContext("background");

Expand Down Expand Up @@ -116,6 +117,7 @@ declare global {
AUDIO_CAPTURE_EVENT: typeof forwardAudioCaptureEvent;

GET_USER_DATA: typeof getUserData;
GET_ME: typeof getMe;
RECORD_LOG: typeof recordLog;
RECORD_ERROR: typeof recordError;
CLEAR_LOGS: typeof clearLogs;
Expand Down Expand Up @@ -194,6 +196,7 @@ export default function registerMessenger(): void {
AUDIO_CAPTURE_EVENT: forwardAudioCaptureEvent,

GET_USER_DATA: getUserData,
GET_ME: getMe,
RECORD_LOG: recordLog,
RECORD_ERROR: recordError,
CLEAR_LOGS: clearLogs,
Expand Down
13 changes: 7 additions & 6 deletions src/components/floatingActions/initFloatingActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@

import { isLoadedInIframe } from "@/utils/iframeUtils";
import { getSettingsState } from "@/store/settings/settingsStorage";
import { getUserData } from "@/background/messenger/api";
import { getMe } from "@/background/messenger/api";
import { transformMeResponse } from "@/data/model/Me";
import { DEFAULT_THEME } from "@/themes/themeTypes";
import { flagOn } from "@/auth/featureFlagStorage";
import { isLinked as getIsLinked } from "@/auth/authStorage";
Expand All @@ -34,18 +35,18 @@ export default async function initFloatingActions(): Promise<void> {
return;
}

const [settings, { telemetryOrganizationId }, isLinked] = await Promise.all([
const [settings, meApiResponse, isLinked] = await Promise.all([
getSettingsState(),
getUserData(),
getMe(),
getIsLinked(),
]);
const meData = transformMeResponse(meApiResponse);

// `telemetryOrganizationId` indicates user is part of an enterprise organization
// See https://github.com/pixiebrix/pixiebrix-app/blob/39fac4874402a541f62e80ab74aaefd446cc3743/api/models/user.py#L68-L68
// Just get the theme from the store instead of using getActive theme to avoid extra Chrome storage reads
// In practice, the Chrome policy should not change between useGetTheme and a call to initFloatingActions on a page
const isEnterpriseOrPartnerUser =
Boolean(telemetryOrganizationId) || settings.theme !== DEFAULT_THEME;
meData?.primaryOrganization?.isEnterprise ||
settings.theme !== DEFAULT_THEME;

const hasFeatureFlag = await flagOn(
FeatureFlags.FLOATING_ACTION_BUTTON_FREEMIUM,
Expand Down
11 changes: 0 additions & 11 deletions src/data/model/Me.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,6 @@ export type Me = {
* The user's primary organization, if they belong to one
*/
primaryOrganization?: MeOrganization;
/**
* The enterprise organization used for telemetry collection. Generally,
* if set, this will be the same as the user's primary organization.
*/
telemetryOrganization?: MeOrganization;
/**
* The partner, controlling theme, documentation links, etc.
*/
Expand Down Expand Up @@ -151,12 +146,6 @@ export function transformMeResponse(response: components["schemas"]["Me"]): Me {
);
}

if (response.telemetry_organization) {
me.telemetryOrganization = transformMeOrganizationResponse(
response.telemetry_organization,
);
}

if (response.partner) {
me.partner = transformUserPartnerResponse(response.partner);
}
Expand Down
5 changes: 5 additions & 0 deletions src/data/model/MeOrganization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ export type MeOrganization = {
* The organization's name.
*/
organizationName: string;
/**
* Whether the organization is an enterprise organization.
*/
isEnterprise: boolean;
/**
* The organization's scope for saving modsmods, if set. A string beginning with "@".
*/
Expand All @@ -60,6 +64,7 @@ export function transformMeOrganizationResponse(
const organization: MeOrganization = {
organizationId: validateUUID(response.id),
organizationName: response.name,
isEnterprise: response.is_enterprise ?? false,
};

if (response.scope) {
Expand Down
3 changes: 0 additions & 3 deletions src/data/service/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import {
import { type components } from "@/types/swagger";
import { dumpBrickYaml } from "@/runtime/brickYaml";
import { isAxiosError } from "@/errors/networkErrorHelpers";
import { getRequestHeadersByAPIVersion } from "@/data/service/apiVersioning";
import { type IntegrationDefinition } from "@/integrations/integrationTypes";
import {
type ModDefinition,
Expand Down Expand Up @@ -420,8 +419,6 @@ export const appApi = createApi({
url: API_PATHS.DEPLOYMENTS,
method: "post",
data,
// @since 1.8.10 -- API version 1.1 excludes the package config
headers: getRequestHeadersByAPIVersion("1.1"),
}),
providesTags: ["Deployments"],
}),
Expand Down
24 changes: 18 additions & 6 deletions src/data/service/apiClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
import { isUrlRelative } from "@/utils/urlUtils";
import createAuthRefreshInterceptor from "axios-auth-refresh";
import { selectAxiosError } from "@/data/service/requestErrorUtils";
import { getURLApiVersion } from "@/data/service/apiVersioning";
import { isAuthenticationAxiosError } from "@/auth/isAuthenticationAxiosError";
import { refreshPartnerAuthentication } from "@/background/messenger/api";

Expand Down Expand Up @@ -69,12 +70,23 @@ async function setupApiClient(): Promise<void> {

apiClientInstance = axios.create({
baseURL: await getBaseURL(),
headers: {
...authHeaders,
// Version 2.0 is paginated. Explicitly pass version, so we can switch the default version on the server
// once clients are all passing an explicit version number
Accept: "application/json; version=1.0",
},
headers: { ...authHeaders },
});

apiClientInstance.interceptors.request.use(async (config) => {
const apiVersion = getURLApiVersion(config.url);

// If apiVersion is the default version (see DEFAULT_API_VERSION), we don't necessarily need the header,
// but let's include it because it has the following benefits:
// - The explicit version header makes troubleshooting easier
// - Allows us to change the default version without breaking clients, see https://github.com/pixiebrix/pixiebrix-app/issues/5060
return {
...config,
headers: {
...config.headers,
Accept: `application/json; version=${apiVersion}`,
},
};
});

// Create auth interceptor for partner auth refresh tokens
Expand Down
33 changes: 25 additions & 8 deletions src/data/service/apiVersioning.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,36 @@
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

// See similar file in the App codebase

import { API_PATHS } from "@/data/service/urlPaths";

// See REST_FRAMEWORK["DEFAULT_VERSION"] in the Django settings
const DEFAULT_API_VERSION = "1.0";

// See REST_FRAMEWORK["ALLOWED_VERSIONS"] in the Django settings
const API_VERSIONS = ["1.0", "1.1", "2.0"] as const;
const API_VERSIONS = [DEFAULT_API_VERSION, "1.1", "2.0"];
export type ApiVersion = (typeof API_VERSIONS)[number];

export function getRequestHeadersByAPIVersion(apiVersion: ApiVersion) {
// The default version doesn't require a header, but pass it anyway to be explicit
// and make troubleshooting easier.
if (API_VERSIONS.includes(apiVersion)) {
return { Accept: `application/json; version=${apiVersion}` };
// Don't include the baseURL in the map keys because the base URL is baked into the axios instance,
// see setupApiClient(), so the URL we're matching against will always be relative.
// Also, the URL must be the full path string. We're not currently doing any regex or substring matching,
// see getURLApiVersion().
const API_VERSION_MAP = new Map<string, ApiVersion>([
// @since 1.8.10 -- excludes the package config
[API_PATHS.DEPLOYMENTS, "1.1"],
// @since 2.0.6 -- includes organization.is_enterprise and excludes telemetry_organization
[API_PATHS.ME, "1.1"],
]);

/**
* Returns the API version for the given URL.
* @param url The URL. Can be undefined because AxiosRequestConfig.url is optional.
*/
export function getURLApiVersion(url: string | undefined): string {
if (!url) {
return DEFAULT_API_VERSION;
}

throw new Error("Unknown API version");
return API_VERSION_MAP.get(url) || DEFAULT_API_VERSION;
}
4 changes: 2 additions & 2 deletions src/data/service/errorService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,11 +157,11 @@ export async function reportToErrorService(
}

const { extensionVersion, ...data } = await selectExtraContext(error);
const { telemetryOrganizationId, organizationId } = await getUserData();
const { organizationId } = await getUserData();

const payload: ErrorItem = {
uuid: uuidv4(),
organization: telemetryOrganizationId ?? organizationId,
organization: organizationId,
class_name: error.name,
message,
deployment: flatContext.deploymentId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ describe("PackageHistory", () => {
organizations: [],
updated_at: "2024-01-26T23:58:12.270168Z" as Timestamp,
verbose_name: "AI Copilot",
share_dependencies: false,
};
axiosMock.onGet(API_PATHS.BRICK_VERSION_MATCH_ANY).reply(200, testVersions);
axiosMock.onGet(API_PATHS.BRICK_MATCH_ANY).reply(200, testPackage);
Expand Down
6 changes: 3 additions & 3 deletions src/hooks/useIsEnterpriseUser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,16 @@
*/

import { useSelector } from "react-redux";
import { selectTelemetryOrganizationId } from "@/auth/authSelectors";
import { selectOrganization } from "@/auth/authSelectors";
import useTheme from "@/hooks/useTheme";
import { DEFAULT_THEME } from "@/themes/themeTypes";

function useIsEnterpriseUser() {
const telemetryOrganizationId = useSelector(selectTelemetryOrganizationId);
const organization = useSelector(selectOrganization);
const {
activeTheme: { themeName },
} = useTheme();
return Boolean(telemetryOrganizationId) || themeName !== DEFAULT_THEME;
return organization?.isEnterprise || themeName !== DEFAULT_THEME;
}

export default useIsEnterpriseUser;
4 changes: 2 additions & 2 deletions src/telemetry/telemetryHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,12 @@ export async function mapAppUserToTelemetryUser(
data: UserData,
): Promise<TelemetryUser> {
const browserId = await getUUID();
const { user, email, telemetryOrganizationId, organizationId } = data;
const { user, email, organizationId } = data;

return {
id: user ?? browserId,
email,
organizationId: telemetryOrganizationId ?? organizationId,
organizationId,
};
}

Expand Down
Loading

0 comments on commit aec9226

Please sign in to comment.