WIP - feature/basic api for app#99
Conversation
|
|
||
| api_routes = [{'name': '/new_event',}, {'name': '/{date}'}] | ||
| api_key = session.query(Token).filter_by(owner_id=user.id).first() | ||
| session.close() |
There was a problem hiding this comment.
Is it necessary? The session is being closed at the end of the get_db
| type="text/javascript" | ||
| src="{{ url_for('static', path='/apiKeyGenerator.js') }}" | ||
| ></script> | ||
| {% endblock %} |
There was a problem hiding this comment.
great work!
remember, tests and documentation are essential
|
|
||
| SQLALCHEMY_DATABASE_URL = os.getenv( | ||
| "DATABASE_CONNECTION_STRING", config.DEVELOPMENT_DATABASE_STRING) | ||
| "DATABASE_CONNECTION_STRING2", config.DEVELOPMENT_DATABASE_STRING) |
| "DATABASE_CONNECTION_STRING", config.DEVELOPMENT_DATABASE_STRING) | ||
| "DATABASE_CONNECTION_STRING2", config.DEVELOPMENT_DATABASE_STRING) | ||
|
|
||
| #pool_pre_ping=True for POSTGRES |
| owner = relationship("User", back_populates="events") | ||
|
|
||
|
|
||
| class Token(Base): |
There was a problem hiding this comment.
I think it could be nice to add a short documentation for this table.
| callAPIKeyRoute("/api/generate_key", { user: user_id, refresh: state }).then( | ||
| (data) => { | ||
| let keyText = document.getElementById("apiKeyHolder"); | ||
| keyText.textContent = "API Key: " + data.key; |
There was a problem hiding this comment.
can't you use f"string {param}" ?
| @@ -1,9 +1,9 @@ | |||
| // Enable bootstrap popovers | |||
| // // Enable bootstrap popovers | |||
There was a problem hiding this comment.
Please revisit this file and remove unneeded code.
|
|
||
| SQLALCHEMY_DATABASE_URL = os.getenv( | ||
| "DATABASE_CONNECTION_STRING", config.DEVELOPMENT_DATABASE_STRING) | ||
| "DATABASE_CONNECTION_STRING2", config.DEVELOPMENT_DATABASE_STRING) |
There was a problem hiding this comment.
Please make sure you don't commit this change :)
|
|
||
| id = Column(String, primary_key=True, index=True) | ||
| owner_id = Column(Integer, ForeignKey("users.id"), nullable=False, unique=True) | ||
| owner = relationship("User", back_populates="token") No newline at end of file |
There was a problem hiding this comment.
As a good practice, we can add expiry_date. If you really into it, read about security in APIs: https://owasp.org/www-project-api-security/
| return JSONResponse(jsonable_encoder({'key': token})) | ||
|
|
||
|
|
||
| @key_gen_router.post('/delete_key') |
There was a problem hiding this comment.
Maybe we should use @key_gen_route.delete?
| api_routes = [{'name': '/new_event',}, {'name': '/{date}'}] | ||
| api_key = session.query(Token).filter_by(owner_id=user.id).first() | ||
| session.close() | ||
| no_api_key = True |
There was a problem hiding this comment.
Maybe we can change these lines to key = getattr(api_key, 'id', None) and remove the no_api_key variable?
| from typing import Optional | ||
|
|
||
|
|
||
| def check_api_key(key: str, session=Depends(get_db)): |
There was a problem hiding this comment.
We should implement rate limit to this check
| data = await request.json() | ||
| if data.get('refresh', False): | ||
| session.query(Token).filter_by(owner_id=data['user']).delete() | ||
| token = secrets.token_urlsafe(32) |
There was a problem hiding this comment.
Great job for using secrets!
| @@ -0,0 +1,8 @@ | |||
| #apiKeyDelete { | |||
| font-size: 0.6rem; | |||
| margin-left: 20px; | |||
| session.commit() | ||
| session.close() | ||
| return JSONResponse(jsonable_encoder({'success': True})) | ||
|
|
| session.add(event) | ||
| session.commit() | ||
| d['id'] = event.id | ||
| session.close() |
4 participants
basic implementation, no docs on front, no tests, no documentation yet
Hey, want some CRs for code