API Authentication #285

병합
elegaanz api-auth 에서 master 로 13 commits 를 머지했습니다 6 년 전
elegaanz 코멘트됨, 6 년 전 (Migrated from github.com)
  • App model
  • API endpoint to register an app
  • ApiToken model
  • OAuth2 endpoint to get a token, from app and user credentials
  • make ApiToken a request guard

Fixes #275

- [x] `App` model - [x] API endpoint to register an app - [x] `ApiToken` model - [x] OAuth2 endpoint to get a token, from app and user credentials - [x] make `ApiToken` a request guard Fixes #275
trinity-1686a 검토됨 6 년 전
trinity-1686a 코멘트됨, 6 년 전
소유자

I think actually the whole /api can be authorized, if I remember well it's denoted by /api/<path..>

I think actually the whole `/api` can be authorized, if I remember well it's denoted by `/api/<path..>`
igalic (Migrated from github.com) 검토됨 6 년 전
igalic (Migrated from github.com) left a comment

👀

👀
@ -0,0 +1,77 @@
use canapi::{Error, Provider};
igalic (Migrated from github.com) 코멘트됨, 6 년 전

should i be watching the canapi repo as well?

should i be watching the canapi repo as well?
elegaanz (Migrated from github.com) 검토됨 6 년 전
@ -0,0 +1,77 @@
use canapi::{Error, Provider};
elegaanz (Migrated from github.com) 코멘트됨, 6 년 전

As you want

As you want
trinity-1686a 검토됨 6 년 전
trinity-1686a left a comment
소유자

I think it could help with readability to have a special type with some parameters, doing by itself something similar to ApiToken.can(), and which could be used as a request guard (so we could do something like fn read_post( [...] , _authorized: Authorization<Read, Post>) and it would deny request without tests to do by ourself)

I think it could help with readability to have a special type with some parameters, doing by itself something similar to ApiToken.can(), and which could be used as a request guard (so we could do something like `fn read_post( [...] , _authorized: Authorization<Read, Post>)` and it would deny request without tests to do by ourself)
trinity-1686a 코멘트됨, 6 년 전
소유자

Just "use apps" is maybe a bit unclear, maybe you should use the full path, and also tell how to use it (Do a post with such data, and such other is optional and....)

token. To do so, use the `/api/v1/apps` API (accessible without a token) to create
Just "use apps" is maybe a bit unclear, maybe you should use the full path, and also tell how to use it (Do a post with such data, and such other is optional and....) ```suggestion token. To do so, use the `/api/v1/apps` API (accessible without a token) to create ```
trinity-1686a 코멘트됨, 6 년 전
소유자

name seems to be required by 36297101f2/plume-models/src/apps.rs (L47)
So I think it should not be an Option

name seems to be required by https://github.com/Plume-org/Plume/blob/36297101f261f473fd03d86a22f46379d125e002/plume-models/src/apps.rs#L47 So I think it should not be an Option
@ -0,0 +1,13 @@
use canapi::Endpoint;
#[derive(Clone, Default, Serialize, Deserialize)]
pub struct AppEndpoint {
trinity-1686a 코멘트됨, 6 년 전
소유자

This feel strange to be at the same time data received from Post (with id, client_id and client_secret ignored, as they must be generated by the server) and data returned by the api (with those same field used, and most likely different than what was originally posted if they where). It should either be 2 different struct or at least a struct with FromForm custom-implemented to ensure that

This feel strange to be at the same time data received from Post (with id, client_id and client_secret ignored, as they must be generated by the server) and data returned by the api (with those same field used, and most likely different than what was originally posted if they where). It should either be 2 different struct or at least a struct with FromForm custom-implemented to ensure that
trinity-1686a 코멘트됨, 6 년 전
소유자

both error message ("Wrong password" and "Unknown user") should probably be merged, for similar reasons as #170

both error message ("Wrong password" and "Unknown user") should probably be merged, for similar reasons as #170
elegaanz (Migrated from github.com) 검토됨 6 년 전
elegaanz (Migrated from github.com) 코멘트됨, 6 년 전

The request should be documented with Swagger (but it is broken for the moment 😢)

The request should be documented with Swagger (but it is broken for the moment :cry:)
elegaanz (Migrated from github.com) 검토됨 6 년 전
@ -0,0 +1,13 @@
use canapi::Endpoint;
#[derive(Clone, Default, Serialize, Deserialize)]
pub struct AppEndpoint {
elegaanz (Migrated from github.com) 코멘트됨, 6 년 전

We can't use FromForm in plume-api, or we would loose all the benefits of canapi.

But I think I may add a Server/Client/Both wrapper type to specify when a field is required and make it easier to check if something has been forgotten.

Or maybe canapi is just a bad idea and we should drop it... 🤔

We can't use `FromForm` in plume-api, or we would loose all the benefits of canapi. But I think I may add a `Server`/`Client`/`Both` wrapper type to specify when a field is required and make it easier to check if something has been forgotten. Or maybe canapi is just a bad idea and we should drop it... :thinking:
elegaanz (Migrated from github.com) 검토됨 6 년 전
elegaanz (Migrated from github.com) 코멘트됨, 6 년 전

I don't know if there is a better way to define this type. If I don't use A and S in its definition, it refuses to build.

I don't know if there is a better way to define this type. If I don't use `A` and `S` in its definition, it refuses to build.
elegaanz (Migrated from github.com) 검토됨 6 년 전
elegaanz (Migrated from github.com) 코멘트됨, 6 년 전

As you can see, these two Options will actually always be None, never Some(A) or Some(S).

As you can see, these two Options will actually always be None, never Some(A) or Some(S).
trinity-1686a 검토됨 6 년 전
trinity-1686a 코멘트됨, 6 년 전
소유자
You can use PhantomData https://doc.rust-lang.org/beta/std/marker/struct.PhantomData.html, it'll probably do the trick Simple example : https://gist.github.com/rust-play/9e51f5b8bb3a915a99d958f5ea982f1a
trinity-1686a 검토됨 6 년 전
trinity-1686a left a comment
소유자

There are just a few quick things that should be changed or discussed, and this will be good to go

There are just a few quick things that should be changed or discussed, and this will be good to go
@ -0,0 +56,4 @@
pub fn can_read(&self, scope: &'static str) -> bool {
self.can("read", scope)
}
trinity-1686a 코멘트됨, 6 년 전
소유자

This is confusing because can take a what set to "read" and a scope set to what. what should probably renamed scope, or something else should be renamed in can

This is confusing because `can` take a `what` set to "read" and a `scope` set to `what`. `what` should probably renamed `scope`, or something else should be renamed in `can`
@ -0,0 +60,4 @@
pub fn can_write(&self, scope: &'static str) -> bool {
self.can("write", scope)
}
trinity-1686a 코멘트됨, 6 년 전
소유자

Same goes here (about variable naming)

Same goes here (about variable naming)
trinity-1686a 코멘트됨, 6 년 전
소유자

Same here (about access without tokens)

Same here (about access without tokens)
trinity-1686a 코멘트됨, 6 년 전
소유자

this kind of endpoint can probably be called without tokens, at least as long as the post is published (and require a valid authorization, from a user having access to the post if it's not)

this kind of endpoint can probably be called without tokens, at least as long as the post is published (and require a valid authorization, from a user having access to the post if it's not)
trinity-1686a 이 변경사항을 승인하였습니다. 6 년 전

리뷰어

trinity-1686a 이 변경사항을 승인하였습니다. 6 년 전
The pull request has been merged as e26a150164.
You can also view command line instructions.

Step 1:

From your project repository, check out a new branch and test the changes.
git checkout -b api-auth master
git pull origin api-auth

Step 2:

Merge the changes and update on Forgejo.
git checkout master
git merge --no-ff api-auth
git push origin master
로그인하여 이 대화에 참여
No reviewers
마일스톤 없음
담당자 없음
참여자 2명
알림
마감일
기한이 올바르지 않거나 범위를 벗어났습니다. 'yyyy-mm-dd'형식을 사용해주십시오.

마감일이 설정되지 않았습니다.

의존성

No dependencies set.

Reference: Plume/Plume#285
불러오는 중...
아직 콘텐츠가 없습니다.