Skip to content

Commit

Permalink
[full-ci] Rework auth handling (#7072)
Browse files Browse the repository at this point in the history
Introduce new authentication architecture

Co-authored-by: Benedikt Kulmann <benedikt@kulmann.biz>
  • Loading branch information
dschmidt and kulmann committed Jul 7, 2022
1 parent 642ab41 commit 434a622
Show file tree
Hide file tree
Showing 179 changed files with 2,659 additions and 1,922 deletions.
18 changes: 9 additions & 9 deletions .yarn/releases/yarn-3.1.0.cjs

Large diffs are not rendered by default.

9 changes: 9 additions & 0 deletions changelog/unreleased/bugfix-logout-deleted-user
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
Bugfix: Logout deleted user on page reload

A user that gets disabled or deleted in the backend now sees an authentication error page upon page reload.
From there they can now properly reach the login page to log in again via a different user (or leave the page entirely).

https://github.com/owncloud/web/issues/4677
https://github.com/owncloud/web/issues/4564
https://github.com/owncloud/web/issues/4795
https://github.com/owncloud/web/pull/7072
13 changes: 13 additions & 0 deletions changelog/unreleased/bugfix-token-renewal
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
Bugfix: Access token renewal

Access token renewals had some flaws which have been fixed as follows:
- OAuth2: access token renewal was not working at all, fixed by switching to authorization code flow with PKCE extension and by migrating from the unmaintained `oidc-client` library to `oidc-client-ts`.
- OpenID Connect: when `offline_access` scope was not requested each token renewal caused a redirect to `/`, which was due to a faulty token update implementation and is fixed.

WARNING: With a setup of ownCloud 10.x.x + oauth2-app older than v0.5.3 this bugfix is a breaking change.
There was a bug in the oauth2-app that required to add the `clientSecret` in the `auth` section of the `config.json` file (although code flow with PKCE doesn't need it).
To mitigate this, please add the `clientSecret` for your `clientId` to the `config.json` file. If the oauth2-app v0.5.3 or newer is
used that's not needed.

https://github.com/owncloud/web/issues/7030
https://github.com/owncloud/web/pull/7072
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
Enhancement: Loading context blocks application bootstrap

The bootstrap architecture has been improved to ensure that the respective context (user or public link)
is fully resolved before applications can finalize their boot process and switch over to rendering their content.
This means that application developers can rely on user data / public link data being loaded (including
e.g. capabilities) when the web runtime triggers the boot processes and rendering of applications.

https://github.com/owncloud/web/issues/7030
https://github.com/owncloud/web/pull/7072
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
Enhancement: Resolve bookmarked public links with password protection

Bookmarks to a public link (e.g. when user navigated into a subfolder and then created a bookmark) or
to an app that was opened from a public link (e.g. photo opened in preview app) now properly resolve
the public link context before loading the bookmarked content. This includes a roundtrip to the
password input prompt for password protected public link, e.g. when a password was set in the first
place, has been changed in the meantime, etc.

https://github.com/owncloud/web/issues/7030
https://github.com/owncloud/web/pull/7072
1 change: 1 addition & 0 deletions dev/docker/oc10.web.config.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
"server": "http://host.docker.internal:8080",
"auth": {
"clientId": "M8W5mo3wQV3VHWYsaYpWhkr8dwa949i4GljCkedHhl7GWqmHMkxSeJgK2PcS0jt5",
"clientSecret": "sqvPYXK94tMsEEVOYORxg8Ufesi2kC4WpJJSYb0Kj1DSAYl6u2XvJZjc3VcitjDv",
"url": "http://host.docker.internal:8080/index.php/apps/oauth2/api/v1/token",
"authUrl": "http://host.docker.internal:8080/index.php/apps/oauth2/authorize",
"logoutUrl": "http://host.docker.internal:8080/index.php/logout"
Expand Down
9 changes: 5 additions & 4 deletions docs/deployments/oc10-app.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ geekdocFilePath: oc10-app.md

{{< toc >}}

The ownCloud Web is being deployed as an app to [ownCloud marketplace](https://marketplace.owncloud.com/) to enable easy early integration into existing ownCloud 10 instances.
ownCloud Web is being deployed as an app to [ownCloud marketplace](https://marketplace.owncloud.com/) to enable easy integration into existing ownCloud 10 instances.
After completing this setup, ownCloud Web will be available on `https://<your-owncloud-server>/index.php/apps/web`.

## Prerequisites
- Running [ownCloud 10 server](https://owncloud.com/download-server/) with version 10.6
- Running [ownCloud 10 server](https://owncloud.com/download-server/) with version 10.8
- Installed [oauth2 app](https://marketplace.owncloud.com/apps/oauth2)
- Command line access to your server

Expand All @@ -33,7 +33,7 @@ You can mark the ownCloud web client as `trusted` by clicking the respective che
{{< /hint >}}

{{< hint >}}
If you use OpenID Connect you instead need to add a new client for ownCloud Web to your identity provider.
If you use OpenID Connect you need to add a new client for ownCloud Web to your identity provider instead.
{{< /hint >}}

## Configure ownCloud 10
Expand Down Expand Up @@ -72,7 +72,8 @@ There are a few config values which need to be set in order for ownCloud Web to
"auth": {
"clientId": "<client-id-from-oauth2>",
"url": "https://<your-owncloud-server>/index.php/apps/oauth2/api/v1/token",
"authUrl": "https://<your-owncloud-server>/index.php/apps/oauth2/authorize"
"authUrl": "https://<your-owncloud-server>/index.php/apps/oauth2/authorize",
"logoutUrl": "https://<your-owncloud-server>/index.php/logout"
},
"apps" : [
"files",
Expand Down
3 changes: 1 addition & 2 deletions packages/web-app-draw-io/src/App.vue
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
</main>
</template>
<script>
import { mapGetters, mapActions } from 'vuex'
import { mapActions } from 'vuex'
import { basename } from 'path'
import qs from 'qs'
import { DateTime } from 'luxon'
Expand All @@ -38,7 +38,6 @@ export default {
currentETag: null
}),
computed: {
...mapGetters(['getToken']),
config() {
const {
url = 'https://embed.diagrams.net',
Expand Down
27 changes: 4 additions & 23 deletions packages/web-app-external/src/App.vue
Original file line number Diff line number Diff line change
Expand Up @@ -32,34 +32,17 @@
</main>
</template>

<script>
<script lang="ts">
import { mapGetters } from 'vuex'
import ErrorScreen from './components/ErrorScreen.vue'
import LoadingScreen from './components/LoadingScreen.vue'
import { DavProperties } from 'web-pkg/src/constants'
import { buildResource } from 'files/src/helpers/resources'
import { computed, unref } from '@vue/composition-api'
import { queryItemAsString, useAppDefaults, useRouteQuery } from 'web-pkg/src/composables'
import { defineComponent } from '@vue/runtime-core'
// FIXME: hacky, get rid asap, just a workaround
// same as packages/web-app-files/src/views/PublicFiles.vue
const unauthenticatedUserReady = async (router, store) => {
if (store.getters.userReady) {
return
}
const publicToken = (router.currentRoute.params.filePath || '').split('/')[0]
const publicLinkPassword = store.getters['Files/publicLinkPassword']
await store.dispatch('loadCapabilities', {
publicToken,
...(publicLinkPassword && { user: 'public', password: publicLinkPassword })
})
store.commit('SET_USER_READY', true)
}
export default {
export default defineComponent({
name: 'ExternalApp',
components: {
Expand Down Expand Up @@ -106,8 +89,6 @@ export default {
}
},
async created() {
await unauthenticatedUserReady(this.$router, this.$store)
this.loading = true
try {
const filePath = this.currentFileContext.path
Expand Down Expand Up @@ -161,5 +142,5 @@ export default {
return buildResource(file)
}
}
}
})
</script>
2 changes: 1 addition & 1 deletion packages/web-app-external/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export default {
routes,
store,
translations,
userReady({ store }) {
ready({ store }) {
store.dispatch('External/fetchMimeTypes')
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ const mockFileInfo = jest.fn(() => ({

const storeOptions = {
getters: {
getToken: jest.fn(() => 'GFwHKXdsMgoFwt'),
configuration: jest.fn(() => ({
server: 'http://example.com/',
currentTheme: {
Expand All @@ -45,7 +44,6 @@ const storeOptions = {
}
}
})),
userReady: () => true,
capabilities: jest.fn(() => ({
files: {
app_providers: [
Expand All @@ -70,6 +68,18 @@ const storeOptions = {
mutations: {
SET_MIME_TYPES: jest.fn()
}
},
runtime: {
namespaced: true,
modules: {
auth: {
namespaced: true,
getters: {
accessToken: jest.fn(() => 'GFwHKXdsMgoFwt'),
isUserContextReady: jest.fn(() => true)
}
}
}
}
}
}
Expand All @@ -92,6 +102,7 @@ const providerSuccessResponseGet = {

describe('The app provider extension', () => {
beforeEach(() => {
// eslint-disable-next-line @typescript-eslint/no-empty-function
jest.spyOn(console, 'error').mockImplementation(() => {})
})

Expand All @@ -100,13 +111,16 @@ describe('The app provider extension', () => {
})

it('should show a loading spinner while loading', async () => {
const makeRequest = jest.fn(() =>
setTimeout(() => {
Promise.resolve({
ok: true,
status: 200
const makeRequest = jest.fn(
() =>
new Promise((resolve, reject) => {
setTimeout(() => {
resolve({
ok: true,
status: 200
})
}, 500)
})
}, 500)
)
const wrapper = createShallowMountWrapper(makeRequest)
await wrapper.vm.$nextTick()
Expand Down
7 changes: 7 additions & 0 deletions packages/web-app-files/src/App.vue
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,19 @@ import Mixins from './mixins'
import { mapActions, mapState, mapMutations } from 'vuex'
import SideBar from './components/SideBar/SideBar.vue'
import { defineComponent } from '@vue/composition-api'
import { usePublicLinkPassword, useStore } from 'web-pkg/src/composables'
export default defineComponent({
components: {
SideBar
},
mixins: [Mixins],
setup() {
const store = useStore()
return {
publicLinkPassword: usePublicLinkPassword({ store })
}
},
computed: {
...mapState('Files/sidebar', {
sidebarClosed: 'closed',
Expand Down
26 changes: 14 additions & 12 deletions packages/web-app-files/src/components/AppBar/CreateAndUpload.vue
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,13 @@ import MixinFileActions, { EDITOR_MODE_CREATE } from '../../mixins/fileActions'
import { buildResource, buildWebDavFilesPath, buildWebDavSpacesPath } from '../../helpers/resources'
import { isLocationPublicActive, isLocationSpacesActive } from '../../router'
import { useActiveLocation } from '../../composables'
import { useRequest, useCapabilityShareJailEnabled } from 'web-pkg/src/composables'
import {
useRequest,
useCapabilityShareJailEnabled,
useStore,
usePublicLinkPassword,
useUserContext
} from 'web-pkg/src/composables'
import { DavProperties, DavProperty } from 'web-pkg/src/constants'
Expand All @@ -138,6 +144,7 @@ export default defineComponent({
setup() {
const instance = getCurrentInstance().proxy
const uppyService = instance.$uppyService
const store = useStore()
onMounted(() => {
const filesSelectedSub = uppyService.subscribe('filesSelected', instance.onFilesSelected)
Expand All @@ -162,13 +169,15 @@ export default defineComponent({
uppyService
}),
...useUploadHelpers(),
...useRequest(),
isPersonalLocation: useActiveLocation(isLocationSpacesActive, 'files-spaces-personal'),
isPublicLocation: useActiveLocation(isLocationPublicActive, 'files-public-files'),
isSpacesProjectsLocation: useActiveLocation(isLocationSpacesActive, 'files-spaces-projects'),
isSpacesProjectLocation: useActiveLocation(isLocationSpacesActive, 'files-spaces-project'),
isSpacesShareLocation: useActiveLocation(isLocationSpacesActive, 'files-spaces-share'),
hasShareJail: useCapabilityShareJailEnabled(),
...useRequest()
publicLinkPassword: usePublicLinkPassword({ store }),
isUserContext: useUserContext({ store })
}
},
data: () => ({
Expand All @@ -177,14 +186,8 @@ export default defineComponent({
fileFolderCreationLoading: false
}),
computed: {
...mapGetters(['getToken', 'capabilities', 'configuration', 'newFileHandlers', 'user']),
...mapGetters('Files', [
'files',
'currentFolder',
'publicLinkPassword',
'spaces',
'selectedFiles'
]),
...mapGetters(['capabilities', 'configuration', 'newFileHandlers', 'user']),
...mapGetters('Files', ['files', 'currentFolder', 'spaces', 'selectedFiles']),
...mapState('Files', ['areFileExtensionsShown']),
mimetypesAllowedForCreation() {
Expand Down Expand Up @@ -748,8 +751,7 @@ export default defineComponent({
conflicts.length
)
const isVersioningEnabled =
!this.publicPage() && this.capabilities.files && this.capabilities.files.versioning
const isVersioningEnabled = this.isUserContext && this.capabilities?.files?.versioning
let translatedMsg
if (isVersioningEnabled) {
Expand Down
25 changes: 9 additions & 16 deletions packages/web-app-files/src/components/AppBar/CreateSpace.vue
Original file line number Diff line number Diff line change
Expand Up @@ -14,25 +14,18 @@
</oc-button>
</template>

<script>
import { mapActions, mapGetters, mapMutations } from 'vuex'
<script lang="ts">
import { mapActions, mapMutations } from 'vuex'
import { buildSpace } from '../../helpers/resources'
import { useStore } from 'web-pkg/src/composables'
import { clientService } from 'web-pkg/src/services'
import { defineComponent } from '@vue/composition-api'
import { useGraphClient } from 'web-client/src/composables'
export default {
export default defineComponent({
setup() {
const store = useStore()
const graphClient = clientService.graphAuthenticated(
store.getters.configuration.server,
store.getters.getToken
)
return { graphClient }
},
computed: {
...mapGetters(['getToken', 'configuration'])
return {
...useGraphClient()
}
},
methods: {
...mapActions(['showMessage', 'createModal', 'hideModal', 'setModalInputErrorMessage']),
Expand Down Expand Up @@ -125,5 +118,5 @@ export default {
})
}
}
}
})
</script>
9 changes: 5 additions & 4 deletions packages/web-app-files/src/components/Search/Preview.vue
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import Vue from 'vue'
import { mapGetters, mapState } from 'vuex'
import { createLocationSpaces } from '../../router'
import path from 'path'
import { useCapabilityShareJailEnabled, useStore } from 'web-pkg/src/composables'
import { useAccessToken, useCapabilityShareJailEnabled, useStore } from 'web-pkg/src/composables'
const visibilityObserver = new VisibilityObserver()
Expand All @@ -49,7 +49,8 @@ export default {
resourceTargetLocation: createLocationSpaces('files-spaces-personal', {
params: { storageId: store.getters.user.id }
}),
resourceTargetLocationSpace: createLocationSpaces('files-spaces-project')
resourceTargetLocationSpace: createLocationSpaces('files-spaces-project'),
accessToken: useAccessToken({ store })
}
},
data() {
Expand All @@ -58,7 +59,7 @@ export default {
}
},
computed: {
...mapGetters(['configuration', 'user', 'getToken']),
...mapGetters(['configuration', 'user']),
...mapState('Files', ['spaces']),
matchingSpace() {
Expand Down Expand Up @@ -96,7 +97,7 @@ export default {
dimensions: ImageDimension.Thumbnail,
server: this.configuration.server,
userId: this.user.id,
token: this.getToken
token: this.accessToken
},
true
)
Expand Down
Loading

0 comments on commit 434a622

Please sign in to comment.