DaftAcademy / daftacademy-python_levelup-spring2020

20 stars 10 forks source link

Wykład 3 zadanie 2: Dlaczego aplikacja nie przechodzi testów? #26

Open Mcrackers opened 4 years ago

Mcrackers commented 4 years ago

Szanowni,

Zwracam się z prośbą o wyjaśnienie, dlaczego poniższy kod nie przechodzi testów? Podczas testowania aplikacji za pomocą curl otrzymuję informację: HTTP/1.1 307 Temporary Redirect Ale podczas assertów na repl.it mam bład 422. W przypadku wpisania niepoprawnego loginu czy hasła wymuszam zwrócenie wyjątku 401: HTTPException(status_code=401) A mimo to w testach na repl.it mam błąd 422.

Poniżej załączam kod:

from fastapi import FastAPI, Response, HTTPException
from hashlib import sha256
from pydantic import BaseModel
from fastapi.responses import JSONResponse
from starlette.responses import RedirectResponse

app = FastAPI()
app.num = 0
app.count = -1
app.users = {"trudnY": "PaC13Nt", "admin": "admin"}
app.secret = "secret"
app.tokens = []
patlist = []

@app.get("/")
def root():
    return {"message": "Hello World during the coronavirus pandemic!"}

@app.get("/welcome")
def welcome_to_the_jungle():
    return {"message": "Welcome to the jungle! We have funny games!"}

@app.post("/login")
def login_to_app(user: str, passw: str, response: Response):
    if user in app.users and passw == app.users[user]:
        s_token = sha256(bytes(f"{user}{passw}{app.secret}", encoding='utf8')).hexdigest()
        app.tokens += s_token
        response.set_cookie(key="session_token",value=s_token)
        response = RedirectResponse(url='/welcome')
        print('logged in')
        return response
    else:
        raise HTTPException(status_code=401)
Calvitium commented 4 years ago

Z tego, co mi się wydaje, nie stosujesz w ogóle BaseAuth, a powinieneś. Ale jednocześnie chyba nie jest to powodem, przez który testy nie przechodzą

mateusz94 commented 4 years ago

@Mcrackers jak wygląda Twoje zapytanie z użyciem curla?

Mcrackers commented 4 years ago

@Calvitium To prawda. Musiałem (chwilowo) zrezygnować z BasicAuth, ponieważ kiedy używam HTTPBasicAuth (z requests.auth), to przy próbie podejrzenia /docs wyrzuca mi błąd: TypeError: Object of type 'HTTPBasicAuth' is not JSON serializable a /docs się w ogóle nie uruchamia. Przypuszczam, że powinienem napiać oddzielną fukncję przyjmującą login i hasło za pomocą BasicAuth, a zwracającą wygenerowany token, ale po kolei...

@mateusz94 przeklejając z terminala:

mariusz@karbenek:~$ curl -X POST "https://mmhomework.herokuapp.com/login?user=admin&passw=admin" -H  "accept: application/json" -v
*   Trying 54.171.40.67...
* TCP_NODELAY set
* Connected to mmhomework.herokuapp.com (54.171.40.67) port 443 (#0)
* ALPN, offering h2
* ALPN, offering http/1.1
* successfully set certificate verify locations:
*   CAfile: /etc/ssl/certs/ca-certificates.crt
  CApath: /etc/ssl/certs
* TLSv1.3 (OUT), TLS handshake, Client hello (1):
* TLSv1.3 (IN), TLS handshake, Server hello (2):
* TLSv1.2 (IN), TLS handshake, Certificate (11):
* TLSv1.2 (IN), TLS handshake, Server key exchange (12):
* TLSv1.2 (IN), TLS handshake, Server finished (14):
* TLSv1.2 (OUT), TLS handshake, Client key exchange (16):
* TLSv1.2 (OUT), TLS change cipher, Client hello (1):
* TLSv1.2 (OUT), TLS handshake, Finished (20):
* TLSv1.2 (IN), TLS handshake, Finished (20):
* SSL connection using TLSv1.2 / ECDHE-RSA-AES128-GCM-SHA256
* ALPN, server did not agree to a protocol
* Server certificate:
*  subject: C=US; ST=California; L=San Francisco; O=Heroku, Inc.; CN=*.herokuapp.com
*  start date: Apr 19 00:00:00 2017 GMT
*  expire date: Jun 22 12:00:00 2020 GMT
*  subjectAltName: host "mmhomework.herokuapp.com" matched cert's "*.herokuapp.com"
*  issuer: C=US; O=DigiCert Inc; OU=www.digicert.com; CN=DigiCert SHA2 High Assurance Server CA
*  SSL certificate verify ok.
> POST /login?user=admin&passw=admin HTTP/1.1
> Host: mmhomework.herokuapp.com
> User-Agent: curl/7.58.0
> accept: application/json
> 
< HTTP/1.1 307 Temporary Redirect
< Connection: keep-alive
< Date: Fri, 24 Apr 2020 13:23:12 GMT
< Server: uvicorn
< Location: /welcome
< Transfer-Encoding: chunked
< Via: 1.1 vegur
< 
* Connection #0 to host mmhomework.herokuapp.com left intact

Być może jest tu rozwiązanie, którego nie dostrzegam

mateusz94 commented 4 years ago

końcówka urla jest niepokojąca: login?user=admin&passw=admin Powinieneś przekazywać to w nagłówku requestu.

Mcrackers commented 4 years ago

@mateusz94 W takim razie: "curl -u username:password http://example.com" ? edit: Tak, już znalazłem.

mateusz94 commented 4 years ago

yhm, tak. Dodaj sobie parametr -vv, wtedy zobaczysz jakie nagłówki wysyłasz i jakie dostajesz

curl -u username:password http://example.com" -vv

Mcrackers commented 4 years ago

@mateusz94 Bardzo dziękuję! Pomogło, zadanie ostatecznie zaliczone, lecz nie rozumiem do końca kilku rzeczy: Dlaczego umieszczenie przekierowania w stylu:

return RedirectResponse(url='/welcome')

nie działa przechodzi testów (kod błędu 422), pomimo, że w konsoli wyraźnie jest odpowiedź:

HTTP/1.1 307 Temporary Redirect

Zastanawia mnie to tym bardziej, że umieszczenie po prostu:

RedirectResponse(url='/welcome')

zalicza częściowo test przekierowania, narzekając jedynie, że status.code jest niewłaściwy (bo koniec końców jest 200, a powinien być 30X). Jeśli dobrze rozumiem, wysyłany do klienta jest tylko ostatni status.code? Ponadto, skoro już przekierowałem na stronę "/welcome" to lokalizacja nie jest przekazywana automatycznie? Dlaczego muszę w headerze informować o lokalizacji?

Obsttube commented 4 years ago
  1. Na wszelki wypadek wspomnę, że kod błędu 422 to "Unprocessable Entity", w takich przypadkach warto sobie wypisać w konsoli heroku szczegóły błędu, np.:

    @app.exception_handler(RequestValidationError)
    async def validation_exception_handler(request: Request, exc: RequestValidationError):
    print(jsonable_encoder({"detail": exc.errors(), "body": exc.body}))
    return JSONResponse(
        status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
        content=jsonable_encoder({"detail": exc.errors(), "body": exc.body}),
    )

    Trzeba wtedy pamiętać o zaimportowaniu:

    from fastapi.encoders import jsonable_encoder
    from fastapi.exceptions import RequestValidationError
    from fastapi.responses import JSONResponse
  2. Co masz na myśli mówiąc "umieszczenie PO PROSTU: RedirectResponse(url='/welcome')"? w sensie nie masz w tej funkcji w ogóle returna? W takim przypadku możesz dodać parametr "status_code=307" do dekoratora. Tzn @app.post("/login", status_code=307) zamiast @app.post("/login")

Obsttube commented 4 years ago

Widzę, że w kodzie masz już response.headers['Location'] = "/welcome". W takim przypadku RedirectResponse(url='/welcome') nic nie zmienia.

Co więcej, nie przypisujesz tego do żadnej zmiennej, więc ta linijka nic nie robi AFAIK. (Pisząc poprzednią odpowiedź zakładałem, że robisz response=RedirectResponse(url='/welcome'), a nie, że w ogóle nie przypisujesz do żadnej zmiennej.)

Choć początkowo używałem po prostu Location, to z ciekawości spróbowałem RedirectResponse. Jak to się mówi "dziwne, u mnie działa" :) (BEZ "Location")

@app.post("/login")
def login(response: Response, session_token: str = Depends(login_check_cred)):
    response = RedirectResponse(url='/welcome')
    response.status_code = status.HTTP_302_FOUND
    response.set_cookie(key="session_token", value=session_token)
    return response

Po przeanalizowaniu twojego kodu okazało się, że błąd 422 wynika z tego, że po prostu nie zaimplementowałeś jeszcze wtedy endpointu /welcome i po prostu fastapi nie wiedziało jak obsłużyć request przeglądarki po przekierowaniu XD

Mcrackers commented 4 years ago

@Obsttube Bardzo dziękuję za wyczerpujące wyjaśnienia! Będzie mi łatwiej odpowiadać, analizując Twoje odpowiedzi od końca:

Po przeanalizowaniu twojego kodu okazało się, że błąd 422 wynika z tego, że po prostu nie zaimplementowałeś jeszcze wtedy endpointu /welcome i po prostu fastapi nie wiedziało jak obsłużyć request przeglądarki po przekierowaniu XD

Nie bardzo rozumiem: endpoint "/welcome" jest obecny w kodzie i występuje przed endpointem "/login". Prawdopodobnie nie widzę czegoś oczywistego, czy mógłbyś to rozwinąć?

Co więcej, nie przypisujesz tego do żadnej zmiennej, więc ta linijka nic nie robi AFAIK. (Pisząc poprzednią odpowiedź zakładałem, że robisz response=RedirectResponse(url='/welcome'), a nie, że w ogóle nie przypisujesz do żadnej zmiennej.)

Choć początkowo używałem po prostu Location, to z ciekawości spróbowałem RedirectResponse. Jak to się mówi "dziwne, u mnie działa" :) (BEZ "Location")

Czy swoje rozwiązania testowałeś na repl.it? Sprawdzając aplikacje lokalnie na komputerze również stwierdzam, że oba rozwiązania działają. Problemy miałem dopiero podczas testów na repl.it.

Bardzo dziękuję za @app.exception_handler(RequestValidationError)!

Obsttube commented 4 years ago

Już 5 dni temu przechodziło testy z location. Wczoraj sprawdzałem RedirectResponse, też przechodziło testy na repl.it.

Hmmm, przejrzałem dokładnie kod na twoim GitHubie jeszcze raz i rzeczywiście masz oraz miałeś w poprzednich commitach "/welcome".

Dlaczego umieszczenie przekierowania w stylu:

return RedirectResponse(url='/welcome')

nie działa przechodzi testów (kod błędu 422), pomimo, że w konsoli wyraźnie jest odpowiedź: 1) Wiesz już dlaczego miałeś wtedy 422? 2) Udało się przejść testy?

Mcrackers commented 4 years ago

@Obsttube Szczerze mówiąc nie wiem. Zacząłem robić kolejne zadania i w którymś commicie poprawiłem (niechcący) błędy z poprzedniego zadania. Ostatecznie zaliczyłem wszyskie zadania. Tak jak było powiedziane na wykładzie, trzeba było przypilnować dobrej implementacji Cookie.

Co prawda, śledząc logi na heroku podczas uruchamiania testów na repl.it odniosłem wrażenie, że aplikacja wciąż nie pracuje stabilnie (w funkcji def check_cookie poprosiłem o printowanie przychodzącego ciasteczka; co ciekawe, ciasteczko nie zawsze się pojawiało) i przechodzi testy z pewnym prawdopodobieństwem (np 8 na 10 testów).

Obsttube commented 4 years ago

Ciekawe. Ale cieszę się, że udało się przejść testy mimo wszystko :)