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

feat: add access denied UI #27335

Merged
merged 4 commits into from
Jan 14, 2025
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion frontend/src/lib/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2742,7 +2742,7 @@ async function handleFetch(url: string, method: string, fetcher: () => Promise<R
error = e
}

apiStatusLogic.findMounted()?.actions.onApiResponse(response, error)
apiStatusLogic.findMounted()?.actions.onApiResponse(response?.clone(), error)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is needed because it calls resonse.json() so downstream functions can't pull out the body response.


if (error || !response) {
if (error && (error as any).name === 'AbortError') {
Expand Down
25 changes: 25 additions & 0 deletions frontend/src/lib/components/AccessDenied/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { useActions } from 'kea'
import { Link } from 'lib/lemon-ui/Link'
import notFoundAstrohog from 'public/not-found-astrohog.png'

import { supportLogic } from '../Support/supportLogic'

export interface AccessDeniedProps {
object: string
}

export function AccessDenied({ object }: AccessDeniedProps): JSX.Element {
const { openSupportForm } = useActions(supportLogic)

return (
<div className="flex flex-col items-center max-w-2xl p-4 my-24 mx-auto text-center">
<img src={notFoundAstrohog} className="w-64 h-64 bg-no-repeat bg-center" />
<h1 className="text-3xl font-bold mt-4 mb-0">Access denied</h1>
<p className="text-sm mt-3 mb-0">
You do not have access to this {object}. Please contact the creator of this {object} or{' '}
<Link onClick={() => openSupportForm({ kind: 'support' })}>support</Link> if you think this is a
mistake.
</p>
</div>
)
}
17 changes: 15 additions & 2 deletions frontend/src/scenes/dashboard/Dashboard.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { LemonButton } from '@posthog/lemon-ui'
import { BindLogic, useActions, useMountedLogic, useValues } from 'kea'
import { AccessDenied } from 'lib/components/AccessDenied'
import { NotFound } from 'lib/components/NotFound'
import { useKeyboardHotkeys } from 'lib/hooks/useKeyboardHotkeys'
import { DashboardEventSource } from 'lib/utils/eventUsageLogic'
Expand Down Expand Up @@ -46,8 +47,16 @@ export function Dashboard({ id, dashboard, placement, themes }: DashboardProps =
}

function DashboardScene(): JSX.Element {
const { placement, dashboard, canEditDashboard, tiles, itemsLoading, dashboardMode, dashboardFailedToLoad } =
useValues(dashboardLogic)
const {
placement,
dashboard,
canEditDashboard,
tiles,
itemsLoading,
dashboardMode,
dashboardFailedToLoad,
accessDeniedToDashboard,
} = useValues(dashboardLogic)
const { setDashboardMode, reportDashboardViewed, abortAnyRunningQuery } = useActions(dashboardLogic)

useEffect(() => {
Expand Down Expand Up @@ -91,6 +100,10 @@ function DashboardScene(): JSX.Element {
return <NotFound object="dashboard" />
}

if (accessDeniedToDashboard) {
return <AccessDenied object="dashboard" />
}

return (
<div className="dashboard">
{placement == DashboardPlacement.Dashboard && <DashboardHeader />}
Expand Down
10 changes: 10 additions & 0 deletions frontend/src/scenes/dashboard/dashboardLogic.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,7 @@ export const dashboardLogic = kea<dashboardLogicType>([
allVariables: values.variables,
}),
resetVariables: () => ({ variables: values.insightVariables }),
setAccessDeniedToDashboard: true,
})),

loaders(({ actions, props, values }) => ({
Expand Down Expand Up @@ -302,6 +303,9 @@ export const dashboardLogic = kea<dashboardLogicType>([
if (error.status === 404) {
return null
}
if (error.status === 403 && error.code === 'permission_denied') {
actions.setAccessDeniedToDashboard()
}
throw error
}
},
Expand Down Expand Up @@ -431,6 +435,12 @@ export const dashboardLogic = kea<dashboardLogicType>([
setPageVisibility: (_, { visible }) => visible,
},
],
accessDeniedToDashboard: [
false,
{
setAccessDeniedToDashboard: () => true,
},
],
dashboardFailedToLoad: [
false,
{
Expand Down
18 changes: 16 additions & 2 deletions frontend/src/scenes/feature-flags/FeatureFlag.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { LemonDialog, LemonSegmentedButton, LemonSkeleton, LemonSwitch } from '@
import { useActions, useValues } from 'kea'
import { Form, Group } from 'kea-forms'
import { router } from 'kea-router'
import { AccessDenied } from 'lib/components/AccessDenied'
import { ActivityLog } from 'lib/components/ActivityLog/ActivityLog'
import { CopyToClipboardInline } from 'lib/components/CopyToClipboard'
import { NotFound } from 'lib/components/NotFound'
Expand Down Expand Up @@ -86,8 +87,16 @@ function focusVariantKeyField(index: number): void {
}

export function FeatureFlag({ id }: { id?: string } = {}): JSX.Element {
const { props, featureFlag, featureFlagLoading, featureFlagMissing, isEditingFlag, newCohortLoading, activeTab } =
useValues(featureFlagLogic)
const {
props,
featureFlag,
featureFlagLoading,
featureFlagMissing,
isEditingFlag,
newCohortLoading,
activeTab,
accessDeniedToFeatureFlag,
} = useValues(featureFlagLogic)
const { featureFlags } = useValues(enabledFeaturesLogic)
const {
deleteFeatureFlag,
Expand All @@ -113,6 +122,7 @@ export function FeatureFlag({ id }: { id?: string } = {}): JSX.Element {
if (featureFlagMissing) {
return <NotFound object="feature flag" />
}

if (featureFlagLoading) {
return (
<div className="space-y-2">
Expand All @@ -124,6 +134,10 @@ export function FeatureFlag({ id }: { id?: string } = {}): JSX.Element {
)
}

if (accessDeniedToFeatureFlag) {
return <AccessDenied object="feature flag" />
}

const tabs = [
{
label: 'Overview',
Expand Down
10 changes: 8 additions & 2 deletions frontend/src/scenes/feature-flags/featureFlagLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,7 @@ export const featureFlagLogic = kea<featureFlagLogicType>([
errors?: any
) => ({ filters, active, errors }),
setScheduledChangeOperation: (changeType: ScheduledChangeOperationType) => ({ changeType }),
setAccessDeniedToFeatureFlag: true,
}),
forms(({ actions, values }) => ({
featureFlag: {
Expand Down Expand Up @@ -449,6 +450,7 @@ export const featureFlagLogic = kea<featureFlagLogicType>([
},
},
],
accessDeniedToFeatureFlag: [false, { setAccessDeniedToFeatureFlag: () => true }],
Copy link
Contributor

Choose a reason for hiding this comment

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

Do any of these cause a flashed screen before the endpoint returns, or are they all covered by page loaders?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah a bit, def room to improve.

propertySelectErrors: [
null as any,
{
Expand Down Expand Up @@ -527,8 +529,12 @@ export const featureFlagLogic = kea<featureFlagLogicType>([
try {
const retrievedFlag: FeatureFlagType = await api.featureFlags.get(props.id)
return variantKeyToIndexFeatureFlagPayloads(retrievedFlag)
} catch (e) {
actions.setFeatureFlagMissing()
} catch (e: any) {
if (e.status === 403 && e.code === 'permission_denied') {
actions.setAccessDeniedToFeatureFlag()
} else {
actions.setFeatureFlagMissing()
}
throw e
}
}
Expand Down
7 changes: 6 additions & 1 deletion frontend/src/scenes/insights/Insight.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { LemonBanner, LemonButton } from '@posthog/lemon-ui'
import { BindLogic, useActions, useMountedLogic, useValues } from 'kea'
import { AccessDenied } from 'lib/components/AccessDenied'
import { DebugCHQueries } from 'lib/components/CommandPalette/DebugCHQueries'
import { isObject } from 'lib/utils'
import { InsightPageHeader } from 'scenes/insights/InsightPageHeader'
Expand Down Expand Up @@ -35,7 +36,7 @@ export function Insight({ insightId }: InsightSceneProps): JSX.Element {
filtersOverride,
variablesOverride,
})
const { insightProps } = useValues(logic)
const { insightProps, accessDeniedToInsight } = useValues(logic)

// insightDataLogic
const { query, showQueryEditor, showDebugPanel } = useValues(insightDataLogic(insightProps))
Expand All @@ -52,6 +53,10 @@ export function Insight({ insightId }: InsightSceneProps): JSX.Element {
}
}

if (accessDeniedToInsight) {
return <AccessDenied object="insight" />
}

return (
<BindLogic logic={insightLogic} props={insightProps}>
<div className="Insight">
Expand Down
31 changes: 20 additions & 11 deletions frontend/src/scenes/insights/insightLogic.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -96,26 +96,34 @@ export const insightLogic: LogicWrapper<insightLogicType> = kea<insightLogicType
metadataUpdate,
}),
highlightSeries: (seriesIndex: number | null) => ({ seriesIndex }),
setAccessDeniedToInsight: true,
}),
loaders(({ actions, values, props }) => ({
insight: [
props.cachedInsight ?? createEmptyInsight(props.dashboardItemId || 'new'),
{
loadInsight: async ({ shortId, filtersOverride, variablesOverride }, breakpoint) => {
await breakpoint(100)
const insight = await insightsApi.getByShortId(
shortId,
undefined,
'async',
filtersOverride,
variablesOverride
)
try {
const insight = await insightsApi.getByShortId(
shortId,
undefined,
'async',
filtersOverride,
variablesOverride
)

if (!insight) {
throw new Error(`Insight with shortId ${shortId} not found`)
}
if (!insight) {
throw new Error(`Insight with shortId ${shortId} not found`)
}

return insight
return insight
} catch (error: any) {
if (error.status === 403 && error.code === 'permission_denied') {
actions.setAccessDeniedToInsight()
}
throw error
}
},
updateInsight: async ({ insightUpdate, callback }, breakpoint) => {
if (!Object.entries(insightUpdate).length) {
Expand Down Expand Up @@ -231,6 +239,7 @@ export const insightLogic: LogicWrapper<insightLogicType> = kea<insightLogicType
return { ...state, dashboards: state.dashboards?.filter((d) => d !== id) }
},
},
accessDeniedToInsight: [false, { setAccessDeniedToInsight: () => true }],
/** The insight's state as it is in the database. */
savedInsight: [
() => props.cachedInsight || ({} as Partial<QueryBasedInsightModel>),
Expand Down
9 changes: 6 additions & 3 deletions frontend/src/scenes/notebooks/Notebook/notebookLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ export const notebookLogic = kea<notebookLogicType>([
selectComment: (itemContextId: string) => ({ itemContextId }),
openShareModal: true,
closeShareModal: true,
setAccessDeniedToNotebook: true,
}),
reducers(({ props }) => ({
isShareModalOpen: [
Expand All @@ -144,6 +145,7 @@ export const notebookLogic = kea<notebookLogicType>([
closeShareModal: () => false,
},
],
accessDeniedToNotebook: [false, { setAccessDeniedToNotebook: () => true }],
localContent: [
null as JSONContent | null,
{ persist: props.mode !== 'canvas', prefix: NOTEBOOKS_VERSION },
Expand Down Expand Up @@ -255,11 +257,12 @@ export const notebookLogic = kea<notebookLogicType>([
'If-None-Match': values.notebook?.version,
})
} catch (e: any) {
if (e.status === 304) {
if (e.status === 403 && e.code === 'permission_denied') {
actions.setAccessDeniedToNotebook()
} else if (e.status === 304) {
// Indicates nothing has changed
return values.notebook
}
if (e.status === 404) {
} else if (e.status === 404) {
return null
}
throw e
Expand Down
7 changes: 6 additions & 1 deletion frontend/src/scenes/notebooks/NotebookScene.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import './NotebookScene.scss'
import { IconInfo, IconOpenSidebar } from '@posthog/icons'
import { LemonButton, LemonTag } from '@posthog/lemon-ui'
import { useActions, useValues } from 'kea'
import { AccessDenied } from 'lib/components/AccessDenied'
import { NotFound } from 'lib/components/NotFound'
import { UserActivityIndicator } from 'lib/components/UserActivityIndicator/UserActivityIndicator'
import { useEffect } from 'react'
Expand Down Expand Up @@ -35,7 +36,7 @@ export const scene: SceneExport = {
export function NotebookScene(): JSX.Element {
const { notebookId, loading } = useValues(notebookSceneLogic)
const { createNotebook } = useActions(notebookSceneLogic)
const { notebook, conflictWarningVisible } = useValues(
const { notebook, conflictWarningVisible, accessDeniedToNotebook } = useValues(
notebookLogic({ shortId: notebookId, target: NotebookTarget.Scene })
)
const { selectNotebook, closeSidePanel } = useActions(notebookPanelLogic)
Expand All @@ -48,6 +49,10 @@ export function NotebookScene(): JSX.Element {
}
}, [notebookId])

if (accessDeniedToNotebook) {
return <AccessDenied object="notebook" />
}

if (!notebook && !loading && !conflictWarningVisible) {
return <NotFound object="notebook" />
}
Expand Down
Loading