Skip to content

Commit

Permalink
fix: Sign up reporting permission error (#1825)
Browse files Browse the repository at this point in the history
  • Loading branch information
JanCizmar authored Jul 26, 2023
1 parent e638119 commit af1dad9
Show file tree
Hide file tree
Showing 26 changed files with 380 additions and 127 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import io.swagger.v3.oas.annotations.Operation
import io.swagger.v3.oas.annotations.tags.Tag
import io.tolgee.component.reporting.BusinessEventPublisher
import io.tolgee.dtos.request.BusinessEventReportRequest
import io.tolgee.dtos.request.IdentifyRequest
import io.tolgee.service.organization.OrganizationRoleService
import io.tolgee.service.security.SecurityService
import io.tolgee.util.Logging
Expand All @@ -17,7 +18,7 @@ import org.springframework.web.bind.annotation.RestController

@RestController
@CrossOrigin(origins = ["*"])
@RequestMapping(value = ["/v2/business-events"])
@RequestMapping(value = ["/v2/public/business-events"])
@Tag(name = "Business events reporting")
class BusinessEventController(
private val businessEventPublisher: BusinessEventPublisher,
Expand All @@ -36,4 +37,15 @@ class BusinessEventController(
Sentry.captureException(e)
}
}

@PostMapping("/identify")
@Operation(summary = "Identifies user")
fun identify(@RequestBody eventData: IdentifyRequest) {
try {
businessEventPublisher.publish(eventData)
} catch (e: Throwable) {
logger.error("Error storing event", e)
Sentry.captureException(e)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class BusinessEventControllerTest : ProjectAuthControllerTest("/v2/projects/") {
@ProjectJWTAuthTestMethod
fun `it accepts header`() {
performPost(
"/v2/business-events/report",
"/v2/public/business-events/report",
mapOf(
"eventName" to "TEST_EVENT",
"organizationId" to testData.userAccountBuilder.defaultOrganizationBuilder.self.id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import io.tolgee.activity.UtmData
import io.tolgee.component.CurrentDateProvider
import io.tolgee.constants.Caches
import io.tolgee.dtos.request.BusinessEventReportRequest
import io.tolgee.dtos.request.IdentifyRequest
import io.tolgee.security.AuthenticationFacade
import io.tolgee.util.Logging
import io.tolgee.util.logger
Expand Down Expand Up @@ -33,7 +34,8 @@ class BusinessEventPublisher(
projectId = request.projectId,
organizationId = request.organizationId,
utmData = getUtmData(),
data = request.data
data = request.data,
anonymousUserId = request.anonymousUserId
)
)
}
Expand All @@ -49,6 +51,17 @@ class BusinessEventPublisher(
)
}

fun publish(event: IdentifyRequest) {
authenticationFacade.userAccountOrNull?.id?.let { userId ->
applicationEventPublisher.publishEvent(
OnIdentifyEvent(
userAccountId = userId,
anonymousUserId = event.anonymousUserId
)
)
}
}

fun publishOnceInTime(
event: OnBusinessEventToCaptureEvent,
onceIn: Duration,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package io.tolgee.component.reporting

import com.posthog.java.PostHog
import io.tolgee.dtos.cacheable.UserAccountDto
import io.tolgee.service.organization.OrganizationService
import io.tolgee.service.project.ProjectService
import io.tolgee.service.security.UserAccountService
Expand Down Expand Up @@ -34,9 +35,21 @@ class BusinessEventReporter(
selfProxied.captureAsync(data)
}

@EventListener
fun identify(data: OnIdentifyEvent) {
val dto = userAccountService.findDto(data.userAccountId) ?: return
postHog?.capture(
data.userAccountId.toString(),
"${'$'}identify",
mapOf(
"${'$'}anon_distinct_id" to data.anonymousUserId,
) + getSetMapOfUserData(dto)
)
}

private fun captureWithPostHog(data: OnBusinessEventToCaptureEvent) {
val id = data.userAccountDto?.id ?: data.instanceId
val setEntry = getSetMapForPostHog(data)
val id = data.userAccountDto?.id ?: data.instanceId ?: data.anonymousUserId
val setEntry = getIdentificationMapForPostHog(data)

postHog?.capture(
id.toString(), data.eventName,
Expand All @@ -53,22 +66,34 @@ class BusinessEventReporter(
* This method returns map with $set property if user information is present
* or if instanceId is sent by self-hosted instance.
*/
private fun getSetMapForPostHog(data: OnBusinessEventToCaptureEvent): Map<String, Map<String, String>> {
private fun getIdentificationMapForPostHog(data: OnBusinessEventToCaptureEvent): Map<String, Any?> {
val setEntry = data.userAccountDto?.let { userAccountDto ->
mapOf(
"${'$'}set" to mapOf(
"email" to userAccountDto.username,
"name" to userAccountDto.name,
)
)
getSetMapOfUserData(userAccountDto)
} ?: data.instanceId?.let {
mapOf(
"${'$'}set" to mapOf(
"instanceId" to it
)
)
} ?: emptyMap()
return setEntry
return setEntry + getAnonIdMap(data)
}

private fun getSetMapOfUserData(userAccountDto: UserAccountDto) = mapOf(
"${'$'}set" to mapOf(
"email" to userAccountDto.username,
"name" to userAccountDto.name,
),
)

fun getAnonIdMap(data: OnBusinessEventToCaptureEvent): Map<String, String> {
return (
data.anonymousUserId?.let {
mapOf(
"${'$'}anon_distinct_id" to data.anonymousUserId,
)
}
) ?: emptyMap()
}

private fun fillOtherData(data: OnBusinessEventToCaptureEvent): OnBusinessEventToCaptureEvent {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,6 @@ data class OnBusinessEventToCaptureEvent(
val utmData: Map<String, Any?>? = null,
val sdkType: String? = null,
val sdkVersion: String? = null,
val data: Map<String, Any?>? = null
val data: Map<String, Any?>? = null,
val anonymousUserId: String? = null,
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package io.tolgee.component.reporting

data class OnIdentifyEvent(
val userAccountId: Long,
val anonymousUserId: String
)
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ data class BusinessEventReportRequest(
@field:NotBlank
var eventName: String = "",

var anonymousUserId: String? = null,

var organizationId: Long? = null,

var projectId: Long? = null,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package io.tolgee.dtos.request

import javax.validation.constraints.NotBlank

data class IdentifyRequest(
@NotBlank
var anonymousUserId: String = ""
)
17 changes: 16 additions & 1 deletion e2e/cypress/common/login.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export const loginViaForm = (username = USERNAME, password = PASSWORD) => {
.type(password)
.should('have.value', password);
cy.gcy('login-button').click();
waitForGlobalLoading();
return waitForGlobalLoading();
};

export const visitSignUp = () => cy.visit(HOST + '/sign_up');
Expand All @@ -77,3 +77,18 @@ export const signUpAfter = (username: string) => {
enableRegistration();
deleteUserSql(username);
};

export function checkAnonymousIdUnset() {
cy.wrap(localStorage).invoke('getItem', 'anonymousUserId').should('be.null');
}

export function checkAnonymousIdSet() {
cy.intercept('POST', '/v2/public/business-events/identify').as('identify');
cy.wrap(localStorage)
.invoke('getItem', 'anonymousUserId')
.should('have.length', 36);
}

export function checkAnonymousUserIdentified() {
cy.wait('@identify').its('response.statusCode').should('eq', 200);
}
16 changes: 16 additions & 0 deletions e2e/cypress/e2e/security/login.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ import {
} from '../../common/apiCalls/common';
import { assertMessage, getPopover } from '../../common/shared';
import {
checkAnonymousIdSet,
checkAnonymousIdUnset,
checkAnonymousUserIdentified,
loginViaForm,
loginWithFakeGithub,
loginWithFakeOAuth2,
Expand All @@ -24,7 +27,13 @@ context('Login', () => {
});

it('login', () => {
checkAnonymousIdSet();

loginViaForm();

checkAnonymousIdUnset();
checkAnonymousUserIdentified();

cy.gcy('login-button').should('not.exist');
cy.xpath("//*[@aria-controls='user-menu']").should('be.visible');
});
Expand All @@ -42,7 +51,12 @@ context('Login', () => {
});

it('login with github', () => {
checkAnonymousIdSet();

loginWithFakeGithub();

checkAnonymousIdUnset();
checkAnonymousUserIdentified();
});
it('login with oauth2', () => {
loginWithFakeOAuth2();
Expand All @@ -51,9 +65,11 @@ context('Login', () => {
it('logout', () => {
login();
cy.reload();
checkAnonymousIdUnset();
cy.xpath("//*[@aria-controls='user-menu']").click();
getPopover().contains('Logout').click();
cy.gcy('login-button').should('be.visible');
checkAnonymousIdSet();
});

it('resets password', () => {
Expand Down
7 changes: 7 additions & 0 deletions e2e/cypress/e2e/security/signUp.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ import {
} from '../../common/apiCalls/common';
import { assertMessage } from '../../common/shared';
import {
checkAnonymousIdSet,
checkAnonymousIdUnset,
checkAnonymousUserIdentified,
fillAndSubmitSignUpForm,
loginWithFakeGithub,
signUpAfter,
Expand Down Expand Up @@ -81,6 +84,7 @@ context('Sign up', () => {
});

it('Signs up without recaptcha', () => {
checkAnonymousIdSet();
visitSignUp();
cy.intercept('/**/sign_up', (req) => {
expect(req.body.recaptchaToken).be.undefined;
Expand All @@ -106,6 +110,7 @@ context('Sign up', () => {
});

it('Signs up', () => {
checkAnonymousIdSet();
cy.intercept('/**/sign_up', (req) => {
expect(req.body.recaptchaToken).have.length.greaterThan(10);
}).as('signUp');
Expand All @@ -124,6 +129,8 @@ context('Sign up', () => {
cy.visit(r.verifyEmailLink);
assertMessage('Email was verified');
});
checkAnonymousIdUnset();
checkAnonymousUserIdentified();
});

it('Signs up without email verification', () => {
Expand Down
2 changes: 0 additions & 2 deletions webapp/src/component/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,11 @@ const Head: FC = () => {
</Helmet>
);
};

export class App extends React.Component {
componentDidCatch(error: Error, errorInfo: React.ErrorInfo): void {
errorActions.globalError.dispatch(error as GlobalError);
throw error;
}

render() {
return (
<>
Expand Down
3 changes: 3 additions & 0 deletions webapp/src/component/MandatoryDataProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import * as Sentry from '@sentry/browser';
import { useGlobalLoading } from './GlobalLoading';
import { PostHog } from 'posthog-js';
import { getUtmParams } from 'tg.fixtures/utmCookie';
import { useIdentify } from 'tg.hooks/useIdentify';

const POSTHOG_INITIALIZED_WINDOW_PROPERTY = 'postHogInitialized';
export const MandatoryDataProvider = (props: any) => {
Expand All @@ -13,6 +14,8 @@ export const MandatoryDataProvider = (props: any) => {
const isLoading = useGlobalContext((v) => v.isLoading);
const isFetching = useGlobalContext((v) => v.isFetching);

useIdentify(userData?.id);

useEffect(() => {
if (config?.clientSentryDsn) {
Sentry.init({
Expand Down
6 changes: 4 additions & 2 deletions webapp/src/component/security/Login/LoginView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ import { AppState } from 'tg.store/index';
import { LoginCredentialsForm } from './LoginCredentialsForm';
import { LoginTotpForm } from './LoginTotpForm';
import { DashboardPage } from 'tg.component/layout/DashboardPage';
import { SPLIT_CONTENT_BREAK_POINT } from '../SplitContent';
import { SPLIT_CONTENT_BREAK_POINT, SplitContent } from '../SplitContent';
import { TranslatedError } from 'tg.translationTools/TranslatedError';
import { CompactView } from 'tg.component/layout/CompactView';
import { SplitContent } from '../SplitContent';
import { LoginMoreInfo } from './LoginMoreInfo';
import { useReportOnce } from 'tg.hooks/useReportEvent';

interface LoginProps {}

Expand All @@ -34,6 +34,8 @@ export const LoginView: FunctionComponent<LoginProps> = (props) => {

const isSmall = useMediaQuery(SPLIT_CONTENT_BREAK_POINT);

useReportOnce('LOGIN_PAGE_OPENED');

const authLoading = useSelector(
(state: AppState) => state.global.authLoading
);
Expand Down
10 changes: 3 additions & 7 deletions webapp/src/component/security/SignUp/SignUpView.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { FunctionComponent, useEffect } from 'react';
import { FunctionComponent } from 'react';
import { T, useTranslate } from '@tolgee/react';
import { useSelector } from 'react-redux';
import { Redirect } from 'react-router-dom';
Expand All @@ -21,7 +21,7 @@ import { GlobalActions } from 'tg.store/global/GlobalActions';
import { useRecaptcha } from './useRecaptcha';
import { useApiMutation } from 'tg.service/http/useQueryApi';
import { SPLIT_CONTENT_BREAK_POINT, SplitContent } from '../SplitContent';
import { useReportEvent } from 'tg.views/organizations/useReportEvent';
import { useReportOnce } from 'tg.hooks/useReportEvent';

export type SignUpType = {
name: string;
Expand Down Expand Up @@ -63,11 +63,7 @@ export const SignUpView: FunctionComponent = () => {
},
});

const reportEvent = useReportEvent();

useEffect(() => {
reportEvent('SIGN_UP_PAGE_OPENED');
}, []);
useReportOnce('SIGN_UP_PAGE_OPENED');

const onSubmit = async (data: SignUpType) => {
const request = {
Expand Down
Loading

0 comments on commit af1dad9

Please sign in to comment.