ocis
ocis copied to clipboard
feat: reva app auth
Description
- [ ] Provide a README.md for that service in the root folder of that service.
- Use CamelCase for section headers.
- [ ] For images and example files used in README.md:
- Create a folder named
md-sources
on the same level where README.md is located. Put all the images and example files referenced by README.md into this folder. - Use absolute references like
https://raw.githubusercontent.com/owncloud/ocis/master/services/<service-name>/md-sources/file
to make the content accessible for both README.md and owncloud.dev bad<img src="https://github.com/owncloud/ocis/blob/master/services/graph/images/mermaid-graph.svg" width="500" />
good<img src="https://raw.githubusercontent.com/owncloud/ocis/master/services/graph/images/mermaid-graph.svg" width="500" />
- Create a folder named
- [ ] If new CLI command are introduced, that command must be described in the README.md.
- [ ] If new global envvars are introduced, the name must start with
OCIS_
. - [ ] Add the service to the makefile in the ocis repo root.
- [ ] Make the service startable for binary and individual startup:
- For single binary add service to
ocis/pkg/runtime
- For individual startup add service to
ocis/pkg/commands
- Add the service config to
ocis-pkg/config/defaultconfig.go
- For single binary add service to
- [ ] If the service is using service accounts, add it to
ocis/pkg/init/init.go
- [ ] Add the service to
.drone.star
to enable CI. - [ ] Inform doc team in an early stage to review the readme AND the environment variables created.
- The description must reflect the behaviour AND usually has a positive code quality impact.
- [ ] Create proper description strings for envvars - see other services for examples, especially when it comes to multiple values. This must include:
- base description, set of available values, description of each value.
- [ ] When suggestable commits are created for text changes and you agree, collect them to a batch and commit them. Do not forget to rebase locally to avoid overwriting the changes made.
- [ ] If new envvars are introduced which serve the same purpose but in multiple services, an additional envvar must be added at the beginning of the list starting with
OCIS_
(global envvar). - [ ] Ensure that a service has a debug port
- [ ] If the new service introduces a new port:
- The port must be added to port-ranges.md and to the README.md file.
- [ ] Make sure to have a function
FullDefaultConfig()
inpkg/config/defaults/defaultconfig.go
of your service. It is needed to create the documentation. - [ ] Add metrics to the code to enable monitoring. See the proxy service for implementation details.
- Plus add a monitoring documentation in the README.md file
Related Issue
- Fixes <issue_link>
Motivation and Context
How Has This Been Tested?
- test environment:
- test case 1:
- test case 2:
- ...
Screenshots (if appropriate):
Types of changes
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] Technical debt
- [ ] Tests only (no source changes)
Checklist:
- [ ] Code changes
- [ ] Unit tests added
- [ ] Acceptance tests added
- [ ] Documentation ticket raised:
Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.
Somehow the app auth interface is not what I am expecting.
GenerateAppPassword
is not taking the user as argument but extracts it from the context (see below)
The function is designed to be called from an authenticated request of the user. We cannot call it from the migration service to generate tokens for any user. We cannot call this function from an cli tool to generate tokens for a user for testing purpose or what ever.
https://github.com/owncloud/ocis/blob/afc6ed1e416c8c1abef832df3e6912b7487cccbd/vendor/github.com/cs3org/reva/v2/pkg/appauth/manager/json/json.go#L140
Furthermore I don't think these scopes are of any help for the calendar and contacts integration: https://github.com/owncloud/ocis/blob/afc6ed1e416c8c1abef832df3e6912b7487cccbd/vendor/github.com/cs3org/go-cs3apis/cs3/auth/provider/v1beta1/resources.pb.go#L77
We need a scope like calendar.Read
files.Read
.... no idea how this should fit in here.
From my understanding these scopes can only be verified in the oCIS proxy. But prove me wrong ....
Scopes are hardcoded in reva - kind of crazy - but okay ....
https://github.com/owncloud/ocis/blob/b6d5cd78f0b5185df1c175246fafde0fa88995e1/vendor/github.com/cs3org/reva/v2/pkg/auth/scope/scope.go#L33
Furthermore I don't think these scopes are of any help for the calendar and contacts integration:
https://github.com/owncloud/ocis/blob/afc6ed1e416c8c1abef832df3e6912b7487cccbd/vendor/github.com/cs3org/go-cs3apis/cs3/auth/provider/v1beta1/resources.pb.go#L77
We need a scope like
calendar.Read
files.Read
.... no idea how this should fit in here.From my understanding these scopes can only be verified in the oCIS proxy. But prove me wrong ....
Please. Let's keep this PR limited to the migration topic for now. As was decided in https://github.com/owncloud/ocis/pull/8522#issuecomment-2092985475
Please. Let's keep this PR limited to the migration topic for now. As was decided in #8522 (comment)
Even then - scopes need to be set properly and I was wondering how this shall work. But I answered this myself already. THX
Somehow the app auth interface is not what I am expecting.
GenerateAppPassword
is not taking the user as argument but extracts it from the context (see below)
:cry:
The function is designed to be called from an authenticated request of the user. We cannot call it from the migration service to generate tokens for any user. We cannot call this function from an cli tool to generate tokens for a user for testing purpose or what ever.
From the cli tool and the migration service you could just use the machineauth
authprovider to impersonate the user for which you want to create an AppToken. Which can be done using the authtype machine:
AuthenticateRequest{
Type: "machine",
ClientId: claim + ":" + value,
ClientSecret: <machineauthsecret>,
}
claim
can be "username", "userid" or "mail".
Alternatively we could enhance the GenerateAppPasswordRequest
and add a userid
field or put the desired userid into the Opaque field of the request. But for now I think going the machineauth
way is simpler and "good enough".
machine auth does not support scopes as far as I can see .... https://github.com/owncloud/ocis/blob/afc6ed1e416c8c1abef832df3e6912b7487cccbd/vendor/github.com/cs3org/reva/v2/pkg/auth/manager/machine/machine.go#L73
but maybe I am missing something.
to sum it up:
- machine auth for migration service
- app auth for calendar, contacts and related
to sum it up:
* machine auth for migration service * app auth for calendar, contacts and related
I guess that was a misunderstanding. I meant you could use machine auth (via the gateway) for getting a token for the target user. Which you can then use with GenerateAppPassword
to create a app token for the user.
Hooking myself in as I need to add a page in the admin docs when merged, else we get build errors there.
seeing following error when trying to call GenerateAppPassword
in create.go ....
rpc error: code = Unimplemented desc = gateway: error calling GenerateAppPassword: rpc error: code = Unimplemented desc = unknown service cs3.auth.applications.v1beta1.ApplicationsAPI
seeing following error when trying to call GenerateAppPassword in create.go ....
Saw a similiar issue before, looks like the connection to the provider doesn't work. Maybe a port issue.
Saw a similiar issue before, looks like the connection to the provider doesn't work. Maybe a port issue.
THX - sorted with @rhafer ...
Quality Gate passed
Issues
3 New issues
0 Accepted issues
Measures
0 Security Hotspots
9.0% Coverage on New Code
0.0% Duplication on New Code
@DeepDiver1975 what needs to be decided to move this forward?
@kobergj will finish it. I am out of this. Thx
Good decision 👍
great that this is moving forward, thx @kobergj !
Quality Gate passed
Issues
4 New issues
0 Accepted issues
Measures
0 Security Hotspots
8.8% Coverage on New Code
0.0% Duplication on New Code