Skip to content
Open
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 package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"type": "module",
"scripts": {
"build": "vue-demi-fix && pnpm vite build",
"build:w": "pnpm build --watch",
"build:w": "VITE_OC_WEB_LOG_LEVEL=debug pnpm build --watch",
"lint": "eslint vite.config.ts '{packages,tests}/**/*.{js,ts,vue}' --color",
"format:check": "prettier . --config packages/prettier-config/index.js --check",
"format:write": "prettier . --config packages/prettier-config/index.js --write",
Expand Down
1 change: 1 addition & 0 deletions packages/web-pkg/src/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ export * from './encodePath'
export * from './objectKeys'
export * from './semver'
export * from './types'
export * from './logger'
33 changes: 33 additions & 0 deletions packages/web-pkg/src/utils/logger.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
type LogLevel = 'debug' | 'info' | 'warn' | 'error'

const priority: Record<LogLevel, number> = {
debug: 0,
info: 1,
warn: 2,
error: 3
}
const envLogLevel = import.meta.env.VITE_OC_WEB_LOG_LEVEL as LogLevel | undefined
const modeLogLevel: LogLevel = import.meta.env.PROD ? 'warn' : 'debug'
const currentLogLevel: LogLevel =
envLogLevel && priority[envLogLevel] !== undefined ? envLogLevel : modeLogLevel

export const logger = {
get level(): LogLevel {
return currentLogLevel
},
isEnabled(level: LogLevel): boolean {
return priority[level] >= priority[currentLogLevel]
},
debug: (...args: unknown[]) => {
if (logger.isEnabled('debug')) console.debug('☁️', ...args)
},
info: (...args: unknown[]) => {
if (logger.isEnabled('info')) console.info('☁️', ...args)
},
warn: (...args: unknown[]) => {
if (logger.isEnabled('warn')) console.warn('☁️', ...args)
},
error: (...args: unknown[]) => {
if (logger.isEnabled('error')) console.error('☁️', ...args)
}
}
10 changes: 10 additions & 0 deletions packages/web-pkg/web.declarations.d.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
// This file must not export or import anything on top-level

/// <reference types="vite/client" />

interface ImportMetaEnv {
readonly VITE_OC_WEB_LOG_LEVEL?: 'debug' | 'info' | 'warn' | 'error'
}

interface ImportMeta {
readonly env: ImportMetaEnv
}

declare module '*?worker' {
const content: string
export default content
Expand Down
37 changes: 22 additions & 15 deletions packages/web-runtime/src/services/auth/authService.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { UserManager } from './userManager'
import { PublicLinkManager } from './publicLinkManager'
import {
logger,
AuthStore,
ClientService,
UserStore,
Expand All @@ -20,6 +21,7 @@ import {
import { unref } from 'vue'
import { Ability } from '@opencloud-eu/web-client'
import { Language } from 'vue3-gettext'
import { sha256 } from '@noble/hashes/sha2.js'
import { PublicLinkType } from '@opencloud-eu/web-client'
import { WebWorkersStore } from '@opencloud-eu/web-pkg'
import { isSilentRedirectRoute } from '../../helpers/silentRedirect'
Expand Down Expand Up @@ -67,7 +69,6 @@ export class AuthService implements AuthServiceInterface {
this.capabilityStore = capabilityStore
this.webWorkersStore = webWorkersStore
}

/**
* Initialize publicLinkContext and userContext (whichever is available, respectively).
*
Expand Down Expand Up @@ -142,7 +143,7 @@ export class AuthService implements AuthServiceInterface {
if (!this.userManager.areEventHandlersRegistered) {
this.userManager.events.addAccessTokenExpired((...args): void => {
const handleExpirationError = () => {
console.error('AccessToken Expired:', ...args)
logger.error('AccessToken Expired:', ...args)
this.handleAuthError(unref(this.router.currentRoute), { forceLogout: true })
}

Expand All @@ -151,7 +152,7 @@ export class AuthService implements AuthServiceInterface {
})

this.userManager.events.addAccessTokenExpiring((...args) => {
console.debug('AccessToken Expiring:', ...args)
logger.debug('AccessToken Expiring:', ...args)
})

this.userManager.events.addUserLoaded(async (user) => {
Expand All @@ -160,19 +161,25 @@ export class AuthService implements AuthServiceInterface {
expiryThreshold: this.accessTokenExpiryThreshold
})

console.debug(
`New User Loaded. access_token: ${user.access_token}, refresh_token: ${user.refresh_token}`
)
logger.debug(`User Loaded`, {
...(user.access_token && {
'access_token (sha256)': sha256(Buffer.from(user.access_token)).slice(0, 8)
}),
...(user.refresh_token && {
'refresh_token (sha256)': sha256(Buffer.from(user.refresh_token)).slice(0, 8)
})
})
Comment on lines +164 to +171
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm I'm not sure if hashing the tokens has any value because you can't decode them. Why not print them as they are since we're only doing this in dev mode?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

console.log can be overwritten which could leak the token to the outside, also services like sentry or similar can access the data:

https://skillstuff.com/if-you-store-tokens-like-this-youre-already-vulnerable/

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh I didn't know, thanks for the info!

Copy link
Contributor

Choose a reason for hiding this comment

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

but why are we logging anything at all then? what benefit does a hashed token bring?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but why are we logging anything at all then? what benefit does a hashed token bring?

I wasn't sure why we log the token either; the only case I can think of is to see that the token refresh is working, that the token is new, and possibly to compare the hash.

Copy link
Contributor

Choose a reason for hiding this comment

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

IMO you can remove it entirely 🙈

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMO you can remove it entirely 🙈

+1, what do you think, should we keep the logger, I prefer it over the native console.*

Copy link
Contributor

Choose a reason for hiding this comment

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

I have the impression that it's a bit over-engineered to be honest

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have the impression that it's a bit over-engineered to be honest

Ok, maybe … 😂, let’s keep the pr open till after my vacation, or just remove the token logs and call it a day 🤗


try {
await this.userManager.updateContext(user.access_token, fetchUserData)
} catch (e) {
console.error(e)
logger.error(e)
await this.handleAuthError(unref(this.router.currentRoute))
}
})

this.userManager.events.addUserUnloaded(() => {
console.log('user unloaded…')
logger.info('user unloaded…')
this.tokenTimerWorker?.resetTokenTimer()
this.resetStateAfterUserLogout()

Expand All @@ -193,7 +200,7 @@ export class AuthService implements AuthServiceInterface {
}
})
this.userManager.events.addSilentRenewError(async (error) => {
console.error('Silent Renew Error:', error)
logger.error('Silent Renew Error:', error)
await this.handleAuthError(unref(this.router.currentRoute))
})

Expand All @@ -213,7 +220,7 @@ export class AuthService implements AuthServiceInterface {
// no userLoaded event and no signInCallback gets triggered
const accessToken = await this.userManager.getAccessToken()
if (accessToken) {
console.debug('[authService:initializeContext] - updating context with saved access_token')
logger.debug('[authService:initializeContext] - updating context with saved access_token')

try {
await this.userManager.updateContext(accessToken, fetchUserData)
Expand All @@ -228,7 +235,7 @@ export class AuthService implements AuthServiceInterface {
this.tokenTimerInitialized = true
}
} catch (e) {
console.error(e)
logger.error(e)
await this.handleAuthError(unref(this.router.currentRoute))
}
}
Expand All @@ -254,11 +261,11 @@ export class AuthService implements AuthServiceInterface {
this.configStore.options.embed.delegateAuthentication &&
accessToken
) {
console.debug('[authService:signInCallback] - setting access_token and fetching user')
logger.debug('[authService:signInCallback] - setting access_token and fetching user')
await this.userManager.updateContext(accessToken, true)

// Setup a listener to handle token refresh
console.debug('[authService:signInCallback] - adding listener to update-token event')
logger.debug('[authService:signInCallback] - adding listener to update-token event')
window.addEventListener('message', this.handleDelegatedTokenUpdate)
} else {
await this.userManager.signinRedirectCallback(this.buildSignInCallbackUrl())
Expand All @@ -270,7 +277,7 @@ export class AuthService implements AuthServiceInterface {
...(redirectRoute.query && { query: redirectRoute.query })
})
} catch (e) {
console.warn('error during authentication:', e)
logger.warn('error during authentication:', e)
return this.handleAuthError(unref(this.router.currentRoute))
}
}
Expand Down Expand Up @@ -382,7 +389,7 @@ export class AuthService implements AuthServiceInterface {
return
}

console.debug('[authService:handleDelegatedTokenUpdate] - going to update the access_token')
logger.debug('[authService:handleDelegatedTokenUpdate] - going to update the access_token')
return this.userManager.updateContext(event.data, false)
}
}
Expand Down